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 fulopattila122 added this to To do in Vanilo v1.0 Mar 17, 2019
@fulopattila122 fulopattila122 self-assigned this Mar 17, 2019
@fulopattila122 fulopattila122 removed this from To Do in Vanilo v1.0 Aug 19, 2019
@fulopattila122 fulopattila122 added this to To Do in Vanilo v1.1 via automation Aug 19, 2019
@Romik90 Romik90 mentioned this issue Aug 26, 2019
@fulopattila122 fulopattila122 removed this from To Do in Vanilo v1.1 Dec 16, 2019
@fulopattila122 fulopattila122 added this to To do in Vanilo Backlog Feb 3, 2020
@fulopattila122 fulopattila122 added this to In progress in Vanilo v2.0 Feb 3, 2020
@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.

Vanilo v2.0 automation moved this from In progress to Done May 24, 2020
@fulopattila122 fulopattila122 moved this from To do to Done in Vanilo Backlog May 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Vanilo v2.0
  
Done
Development

No branches or pull requests

2 participants