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

Rounding error on exactlyDividedBy() #55

Closed
alejandrojsn opened this issue Jan 20, 2021 · 2 comments
Closed

Rounding error on exactlyDividedBy() #55

alejandrojsn opened this issue Jan 20, 2021 · 2 comments

Comments

@alejandrojsn
Copy link

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

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

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.

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