-
Notifications
You must be signed in to change notification settings - Fork 326
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
Add Decimal.min
and .max
#9663
Add Decimal.min
and .max
#9663
Conversation
Decimal.new "12E73" . max (Decimal.new "13E60") . should_equal (Decimal.new "12E73") | ||
Decimal.new "12E73" . max (Decimal.new "13E80") . should_equal (Decimal.new "13E80") | ||
Decimal.new "-12E73" . max (Decimal.new "-13E60") . should_equal (Decimal.new "-13E60") | ||
Decimal.new "-12E73" . max (Decimal.new "-13E80") . should_equal (Decimal.new "-12E73") |
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 are no tests for cases where decimals and non-decimals are intertwined.
I think both
Decimal.new "123" . max 100 . should_equal 123
124 . max (Decimal.new "101") . should_equal 124
would be good to check.
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.
Done
Decimal.new "12" . max (Decimal.new "13") | ||
# => Decimal.new "13" | ||
max : Number -> Number | ||
max self that = if self > that then self else 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.
max self that = if self > that then self else that | |
max self (that : Number) = if self > that then self else that |
It feels like we should also have the checked type ascription here - this should probably make for better widgets in the GUI (at least in future) and will aid type checking.
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.
Done -- actually changed to Decimal
; Number
was wrong.
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 appreciate the small PRs, it makes them so much easier to review! 👍
I'd add a few more tests for this one.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.