brick / math

Arbitrary-precision arithmetic library for PHP
MIT License
1.78k stars 75 forks source link

Rounding error on exactlyDividedBy() #55

Closed alejandrojsn closed 3 years ago

alejandrojsn commented 3 years ago

I had something like this:

BigDecimal::one()->exactlyDividedBy(BigDecimal::of(10));

And I was getting: PHP Fatal error: Uncaught Brick\Math\Exception\RoundingNecessaryException: Rounding is necessary to represent the result of the operation at this scale.

Which I didn't expect because 1/10 doesn't yield infinite decimals.

After some digging, I found out that some piece of my code had changed de bcscale to 8 and Brick\Math\Internal\Calculator\BcMathCalculator::mod was returning '0.00000000' instead of '0', which caused Brick\Math\Internal\Calculator::divRound() to throw the exception since '0.00000000' === '0' is false of course (lines 435, 449).

I found out that since php7.2 (I was using php7.4), the bcmod function accepts a third parameter for the scale, which I think maybe could be used to prevent an error like this? Although I see the library still supports php7.1, so maybe it's not possible

BenMorel commented 3 years ago

Hi, thank you for the bug report. I can confirm that this issue occurs when setting bcscale() to a non-zero value. I'll release a fix soon!

BenMorel commented 3 years ago

Fixed and released as 0.9.2!

The issue was indeed that bcmod() uses a scale starting from PHP 7.2.

CI now tests with and without bcscale(), so any change like this in the future won't go undetected.