search

academe / SagePay-Integration

HTTP Messages for the Sage Pay REST (Pi) gateway.
GNU General Public License v3.0
9 stars 5 forks source link

Extraneous output from var_dump in Money\Amount::withMajorUnit #58

Closed fireatwill closed 7 years ago

fireatwill commented 7 years ago

Found some (left-over, I guess) debug output - line 43 of src/Money/Amount.php:

var_dump(floor($amount)); var_dump(round($amount, 6));

I wondered for quite a while why my output was prefixed with float(1234) float(1234) !!

judgej commented 7 years ago

Whoops. I'll fix that in the morning. If you have a PR ready to accept, that would speed things up. Thanks for reporting it.

judgej commented 7 years ago

I just found one, and decided I would sleep more soundly after removing it.

I was surprise the first in each pair comes out as a float rather than an integer, but it seems floor() returns a float rather than an integer so it can return massive numbers.

For accuracy, you may be better off using the MoneyAmount class rather than Amount as it handles money without resorting to floats. The moneyphp/money composer package would need to be installed to support that.

judgej commented 7 years ago

Closing for now, but please reopen if you find I've missed any more debug statements. Apologies for letting that slip through. 😊

fireatwill commented 7 years ago

Perfect - thanks. I'm new to using git[hub] collaboratively, so was just teaching myself how to create a pull request. Equally embarrassing!

Thanks for the tip re MoneyAmount, I did notice you suggested that in the readme - should have followed your advice! :smile:

judgej commented 7 years ago

I messed upl you are learning. Big difference :-) We all never stop learning though.

The MoneyAmount class uses the moneyphp/money library to handle the actual amounts - formatting, conversion, any arithmetic you may want to perform on it. It is uses widely in many PHP projects. Its central idea is that it doesn't use PHP float values at all. Floats have a nasty habit of creating errors due to rounding of decimals, which can be affected by the precision of the float that is stored. So converting the float £10.70 to pennies (1070) is actually converting the float £10.6999999999 (or something like that) and in some circumstances can end up being formatted as 1069 pennies.

That's what using MoneyAmount tries to avoid, but the fallback Amount class can be used if you have your system precision set appropriately and know exactly what you are doing. Or use another library altogether and write a connectior class like MoneyAmount.

Anyway, that's the background.