-
Notifications
You must be signed in to change notification settings - Fork 623
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
Ensure coerce only applies to multiplication #535
Conversation
@RubyMoney/money-rails-devs any thoughts on this one? |
Related to #405? |
@ct-clearhaus yeah, didn't notice that. Will read through that PR and see what needs to be changed to merge it. |
👍 |
status? |
|
||
def +(other) raise TypeError; end | ||
def -(other) raise TypeError; end | ||
def /(other) raise TypeError; end |
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 would say division is valid for coercion as you would not divide $10
by $2
, but you would divide $10
by 2
.
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 %
modulo operator should also be included with the same treatment as division.
I think this is a great improvement and feel |
Ensure coerce only applies to multiplication
Resurrected @semmons99's commit from last year, that properly handles coercion for non-Money objects. Raises a TypeError in cases like 0 - Money.new(1), where it otherwise produces a wrong result.
Here's some discussion around this — RubyMoney/money-rails#343
P.S. reopened a PR because of the failing TravisCI build. There's an issue with a "thread safe" test that's failing randomly.