-
-
Notifications
You must be signed in to change notification settings - Fork 550
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
rational-numbers: Add ordering to spec #1736
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -422,6 +422,30 @@ | |
"expected": [1, 1] | ||
} | ||
] | ||
}, | ||
{ | ||
"uuid": "bd1eab16-7603-4a77-ae4e-23c930fe39cd", | ||
"description": "Ordering", | ||
"property": "<", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the diffie-hellman exercise, we experimented with using a textual description of the property. This might be useful here too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, didn't know about that exercise. Do you think that format would be better here? I feel like it'd need special handling in generators either way, so I'm not sure if a description like in diffie-hellman has an advantage? It would likely need the comment anyway for extra explanations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would definitely need the comment. I do think a textual property might be somewhat easier to understand. What do you think @SleeplessByte? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Special handling is required either way (see linked-list); I think written out properties are more consistent with the other exercises. Wouldn't care here 🤷🏽♂️🤗 |
||
"comments": [ | ||
"Depending on your language's features, you may have to implement the following operators:", | ||
"=, !=, <=, <, >=, >", | ||
"", | ||
"To test the ordering, it's recommended to iterate through a = -5..5, b = -5..5, c = -5..5, d = -5..5,", | ||
"define rational numbers r = a / b, s = c / d,", | ||
"and compare the result of r $op c,", | ||
"where $op = {=, !=, <=, <, >=, >}", | ||
"to the result of the corresponding floating point comparisons", | ||
"a / b $op c. Analogously, compare r $op s with a / b $op c / d.", | ||
"Skip the iteration step when a == b == 0 or c == d == 0, since 0/0 is undefined.", | ||
"", | ||
"Depending on your implementation of the exercise, you may need to consider", | ||
"division by zero differently in the iteration.", | ||
"", | ||
"This testing method is based on Julia Base tests[1], released under the MIT licence[2].", | ||
"[1]: https://github.com/JuliaLang/julia/blob/52bafeb981bac548afd2264edb518d8d86944dca/test/rational.jl", | ||
"[2]: https://github.com/JuliaLang/julia/blob/52bafeb981bac548afd2264edb518d8d86944dca/LICENSE.md" | ||
] | ||
} | ||
] | ||
} |
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 don't have a better alternative, but this description is fairly minimal. Maybe something like
Rational numbers can be ordered
or something like that?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 name is in the style of the other names, like "absolute value" or "multiplication". Do you think it should be longer anyway?
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.
There is some inconsistency in the test naming. I'm fine with keeping it as is therefore, but let's see what others think.