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 #30 (comment)?

[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 #30 (comment)?

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.

[ravibpatel]

+1 for custom CurrencyProvider. I am using different payment gateways in the app, and they all have different supported currencies and zero decimal currencies. It may help if we can provide different list of currencies to make Money object. It will make it easier to use it with different gateways.

[BenMorel]

Quoting #30 (comment) by @survik1:

Hi. I have recently encountered problem with removed currency as well, when our employees attempted to take a look at HRK invoices. While I understand approach of @tigitz , the solution does not really help with loading of archived invoices and other archived data. We would have to either somehow understand to use different Currency while working with historical currencies, or would have to implement our version of Money and always use that instead of ISO one. Both approaches however feel like workaround of something that should be easier achievable through library itself. After all, it really feels necessary to support work with old data. Furthermore, after release of v1, if that ever happens, every removal of currency would have to lead to new major release which just doesn't feel correct.

It imo leads down to a question whether using static approach is really the best one in the case of currencies (unlike math where there probably can't be any dispute).

The breaking change issue whenever a currency is removed is very valid, in addition to being very annoying to users of this library. I agree that we should re-introduce obsoleted currencies in the ISO currency provider. As long as ISO codes / numeric codes don't conflict with the current ones (hopefully this will never happen), I don't think historical currencies should be treated differently from current currencies. Maybe they just need to be documented as obsolete in the source code. Maybe they deserve a flag in the Currency class.

I'm open to a PR, just be sure to use a trustable, durable source of data that can be referenced in the source code (and ideally scripted like import-currencies.php, if not automated).

[survik1]

Quoting #30 (comment) by @survik1:

Hi. I have recently encountered problem with removed currency as well, when our employees attempted to take a look at HRK invoices. While I understand approach of @tigitz , the solution does not really help with loading of archived invoices and other archived data. We would have to either somehow understand to use different Currency while working with historical currencies, or would have to implement our version of Money and always use that instead of ISO one. Both approaches however feel like workaround of something that should be easier achievable through library itself. After all, it really feels necessary to support work with old data. Furthermore, after release of v1, if that ever happens, every removal of currency would have to lead to new major release which just doesn't feel correct.

It imo leads down to a question whether using static approach is really the best one in the case of currencies (unlike math where there probably can't be any dispute).

The breaking change issue whenever a currency is removed is very valid, in addition to being very annoying to users of this library. I agree that we should re-introduce obsoleted currencies in the ISO currency provider. As long as ISO codes / numeric codes don't conflict with the current ones (hopefully this will never happen), I don't think historical currencies should be treated differently from current currencies. Maybe they just need to be documented as obsolete in the source code. Maybe they deserve a flag in the Currency class.

I'm open to a PR, just be sure to use a trustable, durable source of data that can be referenced in the source code (and ideally scripted like import-currencies.php, if not automated).

Thanks for fast answer. We'll look into it.