Default exchange rates for `Money.sum`

[urcadox]

Hello,

I just noticed a discrepancy in the behavior of Money.to_currency compared to Money. sum.

The former uses Money.ExchangeRates.latest_rates() as a default value, the latter uses an empty map.

Is there any reason not to change Money. sum's default value to Money.ExchangeRates.latest_rates()?

Thanks.

[kipcole9]

Part of me thinks that defaulting to Money.ExchangeRates.latest_rates/0 for Money.to_currency/2 was not such a good idea. Primarily because Money.ExchangeRates.latest_rates/0 will fail miserably if the exchange rates retriever isn't running. The saving grace is that to_currency/2 is explicitly about conversion so the exchange rates are required. For sum/2 , the conversion is implicit and applies only if a money with a different currency is found.

I think there are a couple of choices:

1. Do as you suggest and default the exchange rates to latest_rates/0 for sum/2. That would mean I need to make latest_rates/0 not fail noisily if the retriever isn't running. And then I need to work out what the return value looks like. Because an empty map and an error in retrieval are different things.

2. Actually remove the default from to_currency/2. But of course I don't want to make a breaking change.

I have thought for a while about making the public api (latest_rates/0, historic_rates/0) trap exits from GenServer.call/2` which would make it more friendly. Perhaps I need to do that first.

Thoughts?

[kipcole9]

I've thought about this over night and have pushed a commit to the cldr45 branch that does two things:

1. Do as you suggested and default the rates param for sum/2 to be Money.ExchangeRates.latest_rates/0.
2. Return {:error, reason} from Money.ExchangeRatest.latest_rates/0, historic_ratest/0, last_updated/0 and latest_rates_available?/0 when the retrieval process is not running rather than let the process crash.

The cldr45 branch is due to be published when CLDR 45 is published, typically early in April.

Perhaps you would consider configuring {:ex_money, github: "kipcole9/money", branch: "cldr45"} and give it a test drive and report back before I close the issue?

[urcadox]

Hey,

Great, I'll try to test this out in the next couple of days!