Skip to content
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

"NaN" intentionally a supported Money amount? #143

Closed
coladarci opened this issue Oct 12, 2022 · 2 comments
Closed

"NaN" intentionally a supported Money amount? #143

coladarci opened this issue Oct 12, 2022 · 2 comments

Comments

@coladarci
Copy link

I was surprised to learn today that this works:

iex > Money.new(:USD, "NaN")
Money.new(:USD, "NaN")

I would sort of have expected an error similar to:

iex > Money.new(:USD, "greg")
{:error,
 {Money.InvalidAmountError, "Amount cannot be converted to a number: \"greg\""}}

We have some legacy data floating around that, it turns out have been persisting these due to bad data. We then tried to call Money.to_integer_exp(money) on one of them and that raises:

(We are on Money 5.10.0 at the moment so the stack trace doesn't line up w/ master)

ArithmeticError: bad argument in arithmetic expression
  Module "erlang", in :erlang.*/2
  File "lib/money.ex", line 1947, in Money.to_integer_exp/2

Curious your thoughts on this; we can figure out a work around on our end pretty easily so we aren't blocked here.

Thanks, again, for everything you do!

@LostKobrakai
Copy link
Contributor

LostKobrakai commented Oct 12, 2022

This is probably a sideeffect of the underlying Decimal library, which supports :NaN as well as :inf. Not sure if that's an intended sideeffect or not though.

@kipcole9
Copy link
Owner

@coladarci thanks for the report and definitely a bug (arises for the reasons @LostKobrakai noted). I have published ex_money version 5.12.2 with the following changelog entry:

Bug Fixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants