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

Suggestion: Use Fowler's Money pattern #29

Closed
rogervila opened this issue Dec 12, 2018 · 3 comments
Closed

Suggestion: Use Fowler's Money pattern #29

rogervila opened this issue Dec 12, 2018 · 3 comments

Comments

@rogervila
Copy link

Hello,

In order to avoid currency problems, I suggest using Fowler's Money pattern, via this package: https://github.com/moneyphp/money

What do you think?

@fulopattila122
Copy link
Member

That was our original plan, but I have to say it's not perfect for tax calculations.

I give you an example:

  • You're selling to multiple EU countries
  • You have to calculate prices based on varying EU VAT rates
  • Your base country has 19% VAT rate
  • You want to sell your product for 17 EUR

The net value of the product (without the 19% VAT) is 14.285714286.
If you save it as a money, it'll be: 14.29
If you apply the 19% VAT, the result is: 17.0051 -> 17.01 rounded to money
It means you'll always have 1 cent difference, and using the money pattern you're not able to store the proper net price.

It's also true that many shops don't deal with multiple VAT/sales tax rates and simply store the gross price, thus don't have this problem.

But the scenario I've depicted is not rare at all; this example was a real case.

Therefore, we'll probably go with some kind of a decimal implementation like this: https://github.com/direvus/php-decimal

No final decision was done yet, so I'm looking for arguments.

@fulopattila122
Copy link
Member

Extra: as an alternative solution there's the precise money branch. We've used that solution, but it doesn't seem to ever be the part of the library: moneyphp/money#7

@fulopattila122
Copy link
Member

We're not going to implement this in the core so far.

With Vanilo 2.0 we wanted to replace all the price related numbers to some Decimal implementation. As we headed forward, we were facing multiple difficulties and we had the impression we're causing more problems than we're solving.

Afterward, we have analyzed a couple of similar e-commerce packages (incl. SpreeCommerce, Magento, Sylius, OroCommerce) and have come to the conclusion we'll keep using floats.

Especially, this comment has convinced me to rather deal with those rare rounding issues within their own scope, instead of adding complexity to the primitives. Additionally, the real issues we have faced with the Money pattern and tax calculation (see my comment above) made me revert all the Decimal sprouts in Vanilo 2 and keep using floats.

Whether or not it'll bite us - time will tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants