search

Coder-Spirit / php-bignumbers

A robust library to handle immutable big numbers inside PHP applications
MIT License
131 stars 29 forks source link

ISSUE-60: Fix for division by zero caused by float precision #61

Closed precariouspanther closed 7 years ago

precariouspanther commented 7 years ago

Aims to resolve https://github.com/Litipk/php-bignumbers/issues/60

Note: This fix is based on my assumption that for numbers where the $value_length < $in_scale + 1 the $value_log10_approx should be clamped to zero. This matches current test expectations (and log10 of the sample provided), but may be the incorrect place to address this (perhaps the natural scale for floats is the true source of this issue?)

Proof/Testing

New regression test covering example case and asserting that log10 precision for affected case returns expected value.

Ping @castarco

castarco commented 7 years ago

Have you checked what happens with numbers between 0.0 and 1.0 ? goes everything OK?

precariouspanther commented 7 years ago

@castarco Yeah I did check, but I've updated the test to include a sample to verify that a number between 0.0 and 1.0 still behaves the same as prior to the change. The guard I've added only guards against negative numbers after the log approx. is calculated which would otherwise cause the division by zero (number pow negative = 0). This guard doesn't necessarily correct the behaviour (perhaps the approx log10 behaviour needs to take into account the natural precision vs just the values string length?) however does guard against the div by zero. I've manually verified several cases that we've encountered.

There is some inaccuracies with precision (the sample I've provided performs a 0.175 / 20.00 which should equal 0.00875 but instead rounds to 0.009, and the log10 operation has similar precision loss. However, this mirrors the current master behaviour as well so I set expectations based on this existing behaviour.

Is there some other specific sample cases you'd like my to verify?