brick / money

A money and currency library for PHP
MIT License
1.65k stars 102 forks source link

MoneyBag and custom currencies #25

Closed davewwww closed 4 years ago

davewwww commented 4 years ago

Hi,

i wanted to create a MoneyBag with USD & BTC, but while addig the BTC i get a UnknownCurrencyException.

$moneyBag = new MoneyBag();
$moneyBag->add(Money::of('1234', 'USD'));
$moneyBag->add(Money::of('0.1234', new Currency('BTC', 0, 'Bitcoin', 8)));
UnknownCurrencyException "Unknown currency code: BTC"

Is this a normal/desired behavior or a bug?

https://github.com/brick/money/blob/c87752e6b65666abe91b41c3c41ed4b18186b51e/src/MoneyBag.php#L33

At this line the ISOCurrencyProvider will be called and he doesn't know BTC.

BenMorel commented 4 years ago

Hi, this was a bug indeed, thanks for reporting it!

Fixed in 91f2b5bc35646f172b038e46bb496ad18db59c3c and released in 0.4.5.

davewwww commented 4 years ago

if you fix that problem this way you also have to add your code here: https://github.com/brick/money/blob/ee7c9c78cc2c1ea73469fc020bc0212cf1acce6c/src/Money.php#L182 https://github.com/brick/money/blob/ee7c9c78cc2c1ea73469fc020bc0212cf1acce6c/src/Money.php#L214 https://github.com/brick/money/blob/ee7c9c78cc2c1ea73469fc020bc0212cf1acce6c/src/Money.php#L240 https://github.com/brick/money/blob/ee7c9c78cc2c1ea73469fc020bc0212cf1acce6c/src/Money.php#L619 https://github.com/brick/money/blob/ee7c9c78cc2c1ea73469fc020bc0212cf1acce6c/src/RationalMoney.php#L56

so maybe you move your fix directly to the Currency class: https://github.com/brick/money/blob/ee7c9c78cc2c1ea73469fc020bc0212cf1acce6c/src/Currency.php#L89

BenMorel commented 4 years ago

It's different there. MoneyBag only deals with currency codes, so getAmount() needs to accept custom (string) currency codes as well. Other methods need a Currency instance, so cannot work with custom currency codes directly.

I actually fixed the docblock in a066e30620cbb8a6708e50007dda05b7d0b207d2 to reflect the fact that non-ISO currency codes are acceptable there!