elixirmoney / money

Elixir library for working with Money safer, easier, and fun... Is an interpretation of the Fowler's Money pattern in fun.prog.
https://hex.pm/packages/money/
MIT License
826 stars 139 forks source link

`exponent` is required (but not enforced) when calling `Money.to_string/1` on custom currency #214

Open pdgonzalez872 opened 8 months ago

pdgonzalez872 commented 8 months ago

Hi! Thanks for the lib!

I saw some behaviour that took a moment to figure out why was happening. Turns out I didn't have exponent set up correctly for a custom_currency.

Repro steps:

# Save the below as `repro.exs`
# and run it as `elixir repro.exs`

Mix.install([
  {:money, "~> 1.12"}
])

Application.put_env(:money, :default_currency, :USD)

Application.put_env(:money, :custom_currencies, [
  WILL_WORK: %{name: "WW", symbol: "WW$", exponent: 2},
  WILL_CRY: %{name: "WC", symbol: ":("}
])

ExUnit.start()

defmodule Test do
  use ExUnit.Case

  describe "exponent needs to be present" do
    test "cries down the road when we dont set exponent" do
      # Everything passes
      assert %Money{amount: 123, currency: :USD} = usd = Money.new(123)
      assert %Money{amount: 123, currency: :WILL_WORK} = will_work = Money.new(123, :WILL_WORK)
      assert %Money{amount: 123, currency: :WILL_CRY} = will_cry = Money.new(123, :WILL_CRY)

      assert "$1.23" = Money.to_string(usd)
      assert "WW$1.23" = Money.to_string(will_work)

      # This one doesn't pass
      assert :will_cry = Money.to_string(will_cry)
    end
  end
end

Here is the error:

  1) test exponent needs to be present cries down the road when we dont set exponent (Test)
     repro.exs:21
     ** (ArithmeticError) bad argument in arithmetic expression: nil + 1
     code: assert :will_cry = Money.to_string(will_cry)
     stacktrace:
       :erlang.+(nil, 1)
       (money 1.12.4) lib/money.ex:716: Money.format_number/2
       (money 1.12.4) lib/money.ex:612: Money.to_string/2
       repro.exs:31: (test)

Expected behaviour

Get an error message directing me to the required config, like we have for normal currencies.

Potential solution

Should we verify/raise if the custom_currencies don't have all the required config for the normal/expected usage? Similar to what happens with not having a default currency.

I don't know what else breaks without the required custom_currencies config. This was the only instance I observed but I also haven't done much more with it.

Hope this helps start a conversation. Thanks again for the lib! :heart: