moneyphp / money

PHP implementation of Fowler's Money pattern.
http://moneyphp.org
MIT License
4.56k stars 439 forks source link

Support (multiply|divide) for float numbers #762

Closed ibra-hassan closed 6 months ago

ibra-hassan commented 6 months ago

Hi team, we have a problem with multiplying or dividing by float numbers example:

$value = Money::EUR(800);       // €8.00
$result = $value->multiply(2.5);  // €16.00

Here it will just multiply by 2 only and ignore the float, but if we pass the number as a string it will work fine.

$value = Money::EUR(800);       // €8.00
$result = $value->multiply("2.5");  // €20.00

I'm using this package for a huge project and using multiply and divide functions a lot, this modification will help us to upgrade much more easily.

UlrichEckhardt commented 6 months ago

Just one suggestion up front: Use a linter, like e.g. Psalm or PHPStan. It would have told you that your code isn't clean. Also, use declare(strict_types=1) in your sources.

That said, what should $value->multiply(1.1) do? 1.1 can't be represented in binary without rounding. It's effectively like one third can't be represented exactly in decimal. Not accepting floating point values here actually tries to help you not make mistakes. It might work okay with 1.1 even, but there are values where the conversion to a string doesn't give the expected result. I'm not a maintainer here though, just an interested reader, so don't take this as authoritative opinion.

BTW: Your PR lacks necessary adjustments to the @psalm-param annotations.

HTH

Uli

UlrichEckhardt commented 6 months ago

Just noticed, https://github.com/moneyphp/money/blob/master/doc/concept.rst#type-safety supports some of the above.

frederikbosch commented 6 months ago

No float, it has been explained before.