Removing currencies

[solodkiy]

Hi! I notice in last update that some currencies have been removed from support (ZWL, SLL). I wonder is it correct way to handle? For example some people could have applications which store transactions in these currencies. I understand that now there are replacements, but old transactions stay the same. If application tries load this old transactions by money 'of' construct it will fail. And even more: the only way to do update to this lib version in this scenario will be rewrite All 'of' constructors through the code to add this codes as custom currencies. Because we don't have a way to declare default currency provider.

[BenMorel]

Hi @solodkiy!

One of the aims of this library is to stay up to date with the official ISO currency list, and unfortunately this typically translates to breaking changes, and require a version bump (which will be even more of a problem once this library reaches 1.0, as a simple currency update would theoretically require bumping to 2.0).

I'm not sure how to best handle this: one way to mitigate this would be to add legacy currencies to the ISO currency provider (and as such, keep ZWL and SLL in the present case), but there would still be BC breaks for country-to-default-currency.

And even more: the only way to do update to this lib version in this scenario will be rewrite All 'of' constructors through the code to add this codes as custom currencies. Because we don't have a way to declare default currency provider.

There is an ongoing discussion in #30. I still think that having a global configuration—something like Money::setDefaultCurrencyProvider()—is bad, as your application may not be the only consumer of the library, some of your dependencies might depend on it as well, and start behaving differently if you change the global config.

As it stands, I think that implementing your own currency provider and injecting it via your DI container is the way to go, although I understand that it's easier to do Money::of(1, 'USD') rather than Money::of(1, $this->currencyProvider->get('USD')).

If you have better suggestions, I'm happy to hear them!

[BenMorel]

In particular, could you please share your opinion on https://github.com/brick/money/issues/30#issuecomment-681145552?

[tigitz]

Hi @BenMorel,

I'm currently working at a fintech company where we're building a portfolio management system for our clients.

The point raised by @solodkiy is quite relevant, and we’ve had to account for similar cases in our own system.

For example, some of our clients hold long-term government bonds that were originally issued in Deutsche Mark (DEM). When calculating their current value, since the bonds are still active and interest is being paid, we encountered issues due to the currency switch. While the current value is in euros, the nominal value, which remains important to display, was issued in a now-obsolete currency.

Ultimately, it seems like a matter of defining the scope of the library. Do you want it to support only current, active currencies, or should it also accommodate historical currencies—going back to a reasonable point in modern economic history? I believe the latter would offer greater reliability for enterprise-level projects.

[BenMorel]

Hi @tigitz, thanks for your feedback!

I totally agree that it doesn't hurt to include historical currencies, as long as they don't conflict with current ones (I doubt ISO would allow this anyway).

I should be able to easily reconstruct the history of past currencies from the initial release of brick/money, however going farther than that would require to go backwards through the official list of ISO 4217 amendments, which is quite some work. Unless there is another trustable source somewhere?

That leaves us with the country-to-currency conversion. Maybe we should just make it clear after the 1.0 release that Currency::ofCountry() and ISOCurrencyProvider::getCurrencyForCountry() are not covered by the backwards compatibility promise, and may be updated in minor releases, i.e. 1.x.0? We could also remove this functionality, or extract it in another package, but I think it's still a useful feature of brick/money.

In particular, could you please share your opinion on https://github.com/brick/money/issues/30#issuecomment-681145552?

Looking forward to your opinion on this as well, @tigitz. Historical ISO currencies are not the only issue, people also ask for easy crypto support.

[tigitz]

SIX Financial Information AG is the official Maintenance Agency of these currency codes under ISO 4217 and as such the only recognized, authoritative source on currency code designations

I think it's reliable to use these sources. They provide two lists for both current and historical currencies:

• https://www.six-group.com/dam/download/financial-information/data-center/iso-currrency/lists/list-one.xml
• https://www.six-group.com/dam/download/financial-information/data-center/iso-currrency/lists/list-three.xml

We could set up a CI job to fetch these regularly and generate corresponding PHP files with all the currencies, automatically suggesting a PR when there are updates.

That leaves us with the country-to-currency conversion. Maybe we should just make it clear after the 1.0 release that Currency::ofCountry() and ISOCurrencyProvider::getCurrencyForCountry() are not covered by the backwards compatibility promise, and may be updated in minor releases, i.e. 1.x.0? We could also remove this functionality, or extract it in another package, but I think it's still a useful feature of brick/money.

In line with how SIX distinguishes between current and historic, we can reflect that in our method signatures. Something like ISOCurrencyProvider::getCurrentCurrencyForCountry(): Currency and ISOCurrencyProvider::getHistoricCurrenciesForCountry(): Currency[]. It keeps things flexible.

Looking forward to your opinion on this as well, @tigitz. Historical ISO currencies are not the only issue, people also ask for easy crypto support.

Yeah I'll definitely do because I’ve already explored it and identified two reasonable solutions IMO.