search

mitoteam / jpgraph

JpGraph library composer package with PHP 8.3 support
https://packagist.org/packages/mitoteam/jpgraph
Other
33 stars 5 forks source link

Fix implicit conversion from float to int loses precision exception #6

Closed Pryx closed 2 years ago

Pryx commented 2 years ago

We have encountered this problem in our PHP 8.1 environment. This solution seems to be working fine as it should be what PHP used to do by default.

f1mishutka commented 2 years ago

Hi, Thank you for PR!

Your solution works but the error message is exactly about implicit conversion. It seems that main problem is not rounding after division but implicit conversion from float to int after this.

So I think it would be better to have $k = (int)(($max+$min) / 2); here to convert float to int explicitly. Or event better (to have control on how rounding is done) is to do both rounding and explicit conversion: $k = (int) floor(($max+$min) / 2);

Could you please modify this line in your pull request for me to be able to merge it? Thanks!

P.S.: let me know if you have any other issues or I can release new minor version after merging for your convenience.

Pryx commented 2 years ago

Hi, thanks for the feedback. The solution is technically correct as the exception is IMO primarily about the precision loss due to implicit conversion (can't really lose precision if there are no decimal places), but I see where you are coming from. $k = (int) floor(($max+$min) / 2); seems to be a more explicit version of what I was going for so I pushed it :slightly_smiling_face:

f1mishutka commented 2 years ago

Merged it anyway.

You are right, the reason to show this message is precision loss during types conversion. But PHP does check for precision loss (and thus shows this message) only if there is an implicit type cast. So yes, to suppress error message floor() call is technically enough. But explicit typecast in this case will also speed up things a little. Its like saying to PHP " Don't worry about this, I aware what's going on here ".