Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Floating point issues generates wrong totals #142

Open
bogdanbradeanu opened this issue Dec 14, 2021 · 5 comments
Open

Floating point issues generates wrong totals #142

bogdanbradeanu opened this issue Dec 14, 2021 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@bogdanbradeanu
Copy link

bogdanbradeanu commented Dec 14, 2021

Hello,
This problem touches on all versions because this is a PHP floating point behaviour. Usually when dealing with prices all plugins must multiply prices by 100 and deal with Cents not Dollars + Cents.

Example:
Lets say i create a cart with some items, i attach a global discount of 10% and my order total is 60.40 Eur. I save it to Database.
If i recreate the cart in order to refund some items i want to show the difference between the original total and new total. This is how i spotted the problem. The app was showing me that 60.40 Eur - 60.40 !== 0.0
Screen Shot 2021-12-14 at 11 26 32 AM
Eur = 7.105427357601E-15

($order->total - ($cart->totalFloat() + $transportValue))
$order->total is of type float 60.40
$cart->totalFloat() is of type float 60.40 (but actually, 60.39 because its rounded up)
$transportValue = 10.00 Eur

https://stackoverflow.com/questions/12291042/weird-php-floating-point-behavior

@bumbummen99
Copy link
Owner

bumbummen99 commented Dec 14, 2021

This might be related to #58. AFAIK the issue is that the package does not use absolute values, a solution would be for example to use integers internally or go as far and integrate a lib like MoneyPHP that has a configuration for rounding.

@bumbummen99 bumbummen99 added the help wanted Extra attention is needed label Dec 14, 2021
@stefanocurnis
Copy link

Maybe we could use bcmath.
I use it in my calculator but maybe we can use it also in Cart or CartItem.

Do you want me to try it?

@bumbummen99
Copy link
Owner

bumbummen99 commented Aug 24, 2022

Maybe we could use bcmath. I use it in my calculator but maybe we can use it also in Cart or CartItem.

Do you want me to try it?

Yep we do the same for our cart package. I already started an refactoring to use MoneyPHPwich builds on BCMath. That is pretty complete but what is missing from the refactoring is some way to replace the session storage with database or even better abstract it so any storage can be used - this is essential for API projects where there is simply no session.

Edit: Check:
#145
#148

@stefanocurnis
Copy link

Oh great! Thanks. Do you plan to release both in a single major version? Or release the MoneyPHP refactor first and then the storage abstraction later?

@bumbummen99
Copy link
Owner

Oh great! Thanks. Do you plan to release both in a single major version? Or release the MoneyPHP refactor first and then the storage abstraction later?

I was planning to only do the refactor for MoneyPHP but that also does require a workflow change i.e. working with integers and calculation also changed to use MoneyPHP. Eventually i want all calculation, order and so on to be in another package, this should just be a a cart. But at some point i noticed it does not make sense to do a major version without abstracting cart storage, a lot of times people look for a solution that is compatible with API. It would be preferable to be able to choose any storage one would like and to also make it compatible with mega carts so that it can be paginated - that is why i wanted to use eloquent for the API/Persistent storage and Session for temporary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants