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

The allocate method using integer ratios causes avoidable discrepancies #46

Closed
nammet opened this issue May 26, 2021 · 1 comment
Closed

Comments

@nammet
Copy link

nammet commented May 26, 2021

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
Copy link
Member

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!

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