kipcole9/money

About `Money.ExchangeRatesLite`

Opened this issue · 0 comments

c4710n commented

Hi, Kip. It's me, again.

When planning for the changes of #154, I read the code of Money.ExchangeRates, and I gained more thoughts.

Issues

There are some facts about current ex_money:

  1. it always starts an application - Money.Application.
  2. it always starts the supervision tree in a fixed hierarchy - Money.ExchangeRates.Supervisor.
  3. it can only start one Money.ExchangeRates.Retriever.

Every fact above may cause one possible issue:

  1. ex_money is running as an application, and the configuration is a global, which makes it impossible to use it for different exchange rates services, such as https://openexchangerates.org/ and https://exchangeratesapi.io/, etc. (Although this situation is very rare.)
  2. If we want to move Money.ExchangeRates.Supervisor to another supervision tree, we have to stop it first, then start it again in another place.
  3. Money.ExchangeRates.Retriever is implemented as a GenServer. And, one GenServer will become a bottleneck in the case of large number of messages. In this case, we would typically start multiple GenServers, but the current implementation is not flexible to implement that.

Solution 1

In order to resolve above issues, I tried to implement a new version of Money.ExchangeRates. And, in order to avoid introducing breaking changes, I rename my implementation to Money.ExchangeRatesLite.

When implementing that, I divided the work into four parts:

  1. a multi-adapter,business-agnostic HTTP client. (when implement it, I deliberately avoided using GenServer)
  2. a multi-adapter,business-agnostic cache. (when implement it, I deliberately avoided using GenServer, too. Because ETS is good for concurrency, wrapping it with GenServer weakens its capabilities.)
  3. a multi-adapter retriever
  4. assemble above components into Money.ExchangeRatesLite, and implement it as a GenServer.

To show the code and how to use it, I created a PR and a demo repo:

With this implementation, above issues can be resolved:

  1. it is easy to create different exchange rates module for different exchange rates services.
  2. it is easy start Money.ExchangeRatesLite in any supervisor tree. Because Money.ExchangeRatesLite is a simple GenServer, there's no application or supervisor.
  3. it is easy to create a pool manager for Money.ExchangeRatesLite. Because, again, Money.ExchangeRatesLite is a simple GenServer.

But it has drawbacks:

  • Money.ExchangeRatesLite is scheduling retrieve tasks. Running a GenServer individually works fine. However, when running multiple GenServers, they are not aware of each other and always schedule multiple retrieve tasks. For example, if we create a pool with 5 Money.ExchangeRatesLite workers, there will be 5 scheduled retrieve tasks running in the background.

It is possible to eliminate these drawbacks with more code, but I think that would introduce too many assumptions and make :ex_money less flexible.

So, I don't plan to invest time in this solution any more.

Solution 2

I hope :ex_money provides a lightweight solution, which allow developers to build or choose their own:

  • supervision tree
  • caching mechanism
  • ...

But, how? The method is to provide just functions.

To show the code and how to use it, I created a PR and a demo repo:

This implementation:

  • provides only functions and a helper macro to retrieve exchange rates.
  • doesn't provide cache for exchange rates.
  • doesn't schedule tasks for retrieving exchange rates internally.

Why? Because:

  • functions are the most flexible thing for a library.
  • developers has their own preferred method to build cache. And, for an existing system, it should already include an existing cache mechanism. If :ex_money provide a caching mechanism, developers would have to modify their code to meet the requirements of :ex_money.
  • as mentioned in solution 1, scheduled tasks can cause issues when creating a pool. Therefore, it's better to separate them, and let the developers to decide how to schedule tasks.

What do you think, kip? Is solution2 a good solution from your perspective? Feel free to comment ;).