Confusion caused by inconsistent exchange_rates config options

[c4710n]

Hi, Kip.

First, thank you for your long-standing work.

I have used ex_money for a long time. During the usage I have gained some insights, and I want to share them with you.

---

Confusion caused by inconsistent exchange_rates config options

Now, we have the spec like this:

config :ex_money,
  auto_start_exchange_rate_service: false,
  exchange_rates_retrieve_every: 300_000,
  api_module: Money.ExchangeRates.OpenExchangeRates,
  callback_module: Money.ExchangeRates.Callback,
  preload_historic_rates: nil
  log_failure: :warn,
  log_info: :info,
  log_success: nil

As you can see:

• exchange_rates_retrieve_every is for exchange_rates, and it is prefixed by exchange_rates. That is ok.
• auto_start_exchange_rate_service is for exchange_rates, too. But the term exchange_rate is not at the header.
• api_module / callback_module / preload_historic_rates are all for exchange_rates, but they are not prefixed by exchange_rates.

It is noticeable that there are some inconsistencies here. New developers will definitely wonder:

• why they have different formats?
• whether they apply on the same scope?
• ...

Eliminating these confusions

What about a new spec of config, like:

config :ex_money,
  exchange_rates: [
    api_module: Money.ExchangeRates.OpenExchangeRates,
    callback_module: Money.ExchangeRates.Callback,
    auto_start: false,
    retrieve_every: 300_000,
    preload_historic_rates: nil,
    # ...
  ]

It scopes all the exchange_rates related options, which will eliminate above confusions.

What about your opinion?

I'd like to contribute the implementation of above idea, and I will:

• take care of the warnings of depracations
• polish the docs for this change
• ...(you name it)

But before that, I would like to first confirm if you like this idea or not?

[kipcole9]

I like that idea, thanks for the suggestion. If you're willing to create a PR, it would be gratefully accepted.

As you point out, the code still needs to be backwards compatible. And then include a soft deprecation for this "old" configuration.

[c4710n]

Even if this issue is resolved, it cannot solve some other problems mentioned at #158. Therefore, I choose to close this issue.