brick / money

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

The allocate method using integer ratios causes avoidable discrepancies #46

Closed nammet closed 3 years ago

nammet commented 3 years ago

Consider

use Brick\Money\Money;

$profit = Money::ofMinor('1000', 'GBP');
[$a, $b] = $profit->allocate(37.5, 62.5); // GBP 37.40, GBP 62.60
[$c, $d] = $profit->allocate(62.5, 37.5); // GBP 62.70, GBP 37.30

In both cases it would be preferable if the values to come out were GBP 62.50 and 37.50, regardless of the order.

If the allocate method were to take floats (or perhaps preferably rational numbers) it could implement a test to see if the allocation yields a remainder. If so, convert to integer.

BenMorel commented 3 years ago

Hi, the main issue is that your floats are casted to int, so you're actually passing 37 and 62, respectively. PHP does not even complain in this case, which is a pity IMO. Now because of the spread of the remainder on the first monies, you get a different result when the parameters are passed in a different order, this is expected.

Now if you want consistent results not depending on the order of the ratios, you can use allocateWithRemainder():

use Brick\Money\Money;

$profit = Money::ofMinor(1000, 'GBP');

[$a, $b, $r1] = $profit->allocateWithRemainder(37, 62); // 3.73, 6.26, 0.01
[$c, $d, $r2] = $profit->allocateWithRemainder(62, 37); // 6.26, 3.73, 0.01

I would consider a PR to accept non-integer ratios, by the way!