-
-
Notifications
You must be signed in to change notification settings - Fork 365
Add Saddle Points test generator #463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- add SaddlePoint test generator - add multidimensional value formater
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that some refactoring is probably wise. Feel free to refactor some functionality into separate methods. The rest of it looks great.
generators/Exercises/SaddlePoints.cs
Outdated
|
||
canonicalDataCase.Properties["input"] = (canonicalDataCase.Properties["input"] as JArray).ToObject<int[,]>(); | ||
|
||
var array = (canonicalDataCase.Expected as Array); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can lose the parentheses here
{ | ||
var input = new[,] | ||
{ | ||
{ 9, 8, 7 }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation is slight off (also in the other cases), I would expect the following:
var input = new[,]
{
{ 9, 8, 7 },
};
- cleanup test generator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you also want to do some refactoring. If you'd like to do that in a separate PR, that would be fine too.
I'll do refactoring in separate PR |
Merged. Thanks again! 🎉 |
This reference #421 issue.
What do you think about those changes - extension of ValueFormatter? :) This switch(val) in Format method gets bigger and bigger so maybe some refactoring needs to be done here (at least split to methods).