Call simplify before toFloat conversion

[olsavmic]

Calling $rationalNumber->toFloat() may result in a huge loss of precision due to the limits of floating point representation which can be prevented (not every time, but in a lot of real-world cases) by calling ->simplified() first.

Such loss of precision caused by a very large nominator or denominator may be actually (and unexpectedly) way larger than if regular 64bit floating numbers were used for all math operations instead (assuming some real-world numeric range). Even with regular numeric range, float overflow may easily occur.

We ran into this issue in production quite a few times in the legacy parts where we did not migrate yet to using BigRational and we perform the float conversion.

Therefore I'd like to suggest calling ->simplify every time before ->toFloat() conversion. I understand that the operation does not come for free, I'd like to hear your opinion :)

Thank you for this otherwise excellent library!

[BenMorel]

Hi, I think this makes a lot of sense! Could you please add a test that would fail without simplify() and succeed with it?

[olsavmic]

Hi, I added two test cases showing the issue. Seems like I was mistaken with the "Such loss of precision caused by a very large nominator or denominator may be actually (and unexpectedly) way larger..." part (and the error is always within the scale of PHP_FLOAT_EPSILON) but the test case with float overflow shows a real-world scenario that can easily occur and can be prevented. :)

[BenMorel]

Thanks, could you please avoid rewriting the coding style as it makes reading the diff tedious. FYI we’re working on imposing a coding style on the project and while some or your changes are relevant, they’ll be applied in a separate step.

[olsavmic]

Of course, sorry for that!

[BenMorel]

Thank you! Released as 0.10.2.