Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non decimal currencies are not properly supported #51

Open
tigitz opened this issue Aug 21, 2021 · 3 comments
Open

Non decimal currencies are not properly supported #51

tigitz opened this issue Aug 21, 2021 · 3 comments

Comments

@tigitz
Copy link

tigitz commented Aug 21, 2021

As I was helping a friend designing a money library in javascript (dinero.js), I tried to find how non decimal currencies were solved in PHP libraries, as it's the language I'm most familiar. I ended up stumbling upon yours.

Problem is point 4 of the "Falsehoods programmers believe about prices" list.

We struggled on supporting non decimal currencies and I realised brick\money currently doesn't support them at all.

As stated here, 1 Ariary (unit) = 5 iraimbilanja (minor unit).

However brick\money gives 1 Ariary (unit) = 100 iraimbilanja (minor unit)

\Brick\Money\Money::of(1, 'MGA')->getMinorAmount(); // 100

Non decimal currencies has some major implications, formatters for example won't be able to represent the whole amount in a decimal fashion.

Here's some content that could contribute to your own solution:
https://v2.dinerojs.com/docs/core-concepts/currency#currency-base
dinerojs/dinero.js#294 (comment)

@BenMorel
Copy link
Member

BenMorel commented Aug 24, 2021

Hi, thanks for bringing this to my attention!

brick/money uses the official ISO 4217 currency list, available as XML here.

This list defines MGA as:

<CcyNtry>
    <CtryNm>MADAGASCAR</CtryNm>
    <CcyNm>Malagasy Ariary</CcyNm>
    <Ccy>MGA</Ccy>
    <CcyNbr>969</CcyNbr>
    <CcyMnrUnts>2</CcyMnrUnts>
</CcyNtry>

As you can see, it defines the currency minor units as 2 decimals, which is the value used in brick/money, and why you get 1 MGA = 100 minor units.

I understand that this may not be of practical use, but if ISO defines it this way, there is probably a reason, and I'm not sure if it would make sense for me to start deviating from the values provided by the ISO standard, at least for the default behaviour: maintaining a up-to-date list of currencies could become a nightmare.

Now that doesn't mean that the library should not allow a different usage that deviates from what the ISO standard tells us!

Regarding the subdivision 1 ariary = 5 iraimbilanja, would this be just an example of cash rounding? As per the example in the README, the Swiss Franc has a minor unit of 0.01, but only coins of 0.05, and the library does provide support for this through contexts.

For MGA, this could look like:

use Brick\Money\Money;
use Brick\Money\Context\CashContext;
use Brick\Math\RoundingMode;

$money = Money::of(10, 'MGA', new CashContext(20)); // MGA 10.00
$money->dividedBy(3, RoundingMode::DOWN); // MGA 3.20
$money->dividedBy(3, RoundingMode::UP); // MGA 3.40

Does that make any sense?

Could you give my the expected formatted output for 1 ariary + 2 iraimbilanja, for example? (what brick/money would output, probably erroneously, as 1.40 ariary)

The approach uses by dinero.js is very interesting though, I'll look into it. Quoting their docs:

the British pound sterling was divided into 20 shillings, and each shilling into 12 pence.

This is definitely not possible using brick/money's current architecture (definitely possible using the underlying library brick/math, though; just a different architecture for the money part).

@tigitz
Copy link
Author

tigitz commented Aug 24, 2021

Indeed, I'm aware the list shows "2" in this column but it's here just for the sake of filling this column with something. It doesn't mean it's a decimal currency with a 1:100 ratio as Wikipedia says

I think it's reasonable to implement the two current exceptions that exists if you want the library to properly support ISO_4217 currencies. Non decimal currencies circulating tend to disappear anyway so there's a high chance it will be a one off job without opening ways to any future maintenance burden.

I don't think cash context is the solution as it's unrelated to the fact that formating money to
10.2 MGA doesn't make sense. The dot implies a decimal which MGA currency hasn't.

Could you give me the expected formatted output for 1 ariary + 2 iraimbilanja

Well this is the problem we tried to solve in dinero and we settled on having 2 output formats.

An "amount and currency" one which is the default as it covers all the type of currencies, even one with multiple minor units. That would give 1 ariary 2 iraimbilanja

And a second "decimal" output that you explicitly need to call like toDecimal that would throw an exception in case currency cannot be represented in base 10.

If you're open to the idea I could try implementing it.

@BenMorel
Copy link
Member

@tigitz Thank you for the detailed explanation. If we can trust the Wikipedia article that the Mauritanian ouguiya and the Malagasy ariary are the only 2 currencies that are non-decimal, then I'm open to make an exception for these (if you find another source for this information, that would make me a bit more confident though). As I said I'm slightly worried to override the values provided by the ISO standard, but if that is the only way to make the library useful to users of these currencies, I'm in.

(That being said, I would really like to understand why ISO did express minor units in terms of "number of decimals", vs "number of minor units". If they said "USD = 100", "MGA = 5", that would have solved the problem for non-decimal currencies, and prevented them from propagating false information. But that's another story I guess.)

Please feel free to start working on this, but be aware that this a rather large work with a large potential for breaking changes, so the architecture should probably be discussed first. In particular, this means that we don't represent the value as a BigDecimal anymore: maybe as a BigRational, maybe a BigNumber whose type depends on the context, maybe as two numbers (major + minor units), or maybe as a BigInteger that represents the total number of minor units. Questions: how do we provide backwards compatibility with the fact that Money allows you to specify a custom number of decimals? Should we switch the precision from a number of decimal places to a number of minor units? Should we have different Context classes that abstract these different use cases (decimal vs non-decimal) to make them more explicit to the user? What should getAmount() return?

That's a lot of questions. Anyway, ideas (and implementations) welcome! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants