[CoreShop2] Store money values as integers

[Yivan]

Hello,

I see the integer price branch, but as we cannot comment branch , i open a issue just to post a comment. This is great to forget about float/double for financial datas : ) Thanks a lot!

But why not having used MySQL DECIMAL field ? It works great and make more easy to handle as there is no integer (from/to) formating to do.

[dpfaffenbauer]

I thought about that too and didn't tried it yet. The branch is more like to test how easy/hard it is and to see what side-effects integers bring with them. Other systems (like magento or prestashop) use decimal as well, but newer ones use integers (sylius). Since the ecommerce framework is gonna use integer as well, I think we should stick with integers?

I am also testing/trying/experimenting to implement a Bridge between CoreShop and the ecommerce framework. This would allow us to use CoreShop stuff with the Framework, therefore integer would be the better choice, what do you think?

[fashxp]

we thought about decimal as well, but then we decided if we do it, we do it properly and go for integer. because with decimal in the database, you still have the problem of floats in php. we don't know exactly how we gonna do it yet, but definitely before launch of pimcore 5 ;-)

[fashxp]

this bridge thing sounds interesting - if you need any help/input let us know

[dpfaffenbauer]

@fashxp Help would be good, since I barely know the framework and how it's supposed to work. I already started something, but lets discuss this over here #176

[dpfaffenbauer]

@fashxp regarding integers: Quite easy, you need to have a custom object-tag where users can input normal decimal numbers and store those as integers in the database (https://github.com/coreshop/CoreShop/blob/integer-prices/src/CoreShop/Bundle/MoneyBundle/CoreExtension/Money.php). All the calculations within your framework should stay the same.

You need to take care for price-rules though, CoreShop solves this issue using Symfony Forms and From Transformers (https://github.com/coreshop/CoreShop/tree/integer-prices/src/CoreShop/Bundle/MoneyBundle/Form).

Since CoreShop uses the form only when validating and saving data, you also need to recalculate prices when displaying your custom JS stuff: https://github.com/coreshop/CoreShop/blob/integer-prices/src/CoreShop/Bundle/OrderBundle/Resources/public/pimcore/js/cart/pricerules/actions/discountAmount.js#L23. This could be solved with a custom ExtJS Form field (which I am probably gonna implement for CoreShop) to display integers values as "money" values.

[fashxp]

yes it is quite easy, if there weren't all those existing installations ;-)

[dpfaffenbauer]

That's right ;)

Still could be done using a config flag and check that inside the tag and JavaScript stuff.

[fashxp]

that is the plan right now, yes. keep the old stuff as it is, and use integer for new projects.

[Yivan]

Thanks for detailled answer. Integer seems a good choice if we could have a bridge : ) I like very much this idea!

[dpfaffenbauer]

merged

[Yivan]

@dpfaffenbauer it seems that Pimcore 5 will be storing in database as DECIMAL(): https://github.com/pimcore/pimcore/pull/1701 , see:

Implement price handling as value object storing values as integer internally. Prices will be saved into a DECIMAL() DB column instead of a double.

I didn't check the code but is think PHP side calculation are made in integer and db storage as DECIMAL(). So there is not rounding error on the PHP side, and we can make use of SQL calcul function directly on the database. Will CoreShop2 working the same (class utility working in integer for prices and storage as DECIMAL()) ?

[Yivan]

Just for reference for people interested on this subject how Pimcore 5 handle it: https://github.com/pimcore/pimcore/commit/7aff3d9e0256788d65dace52b8e43b11ac685473

[dpfaffenbauer]

@Yivan Don't think so, I don't see any advantages in using a decimal over an integer. Since CoreShop calculates internally using integer, why not store data as integer then as well?

To be honest, I thought about a similar solution as Pimcore did using MoneyPHP (https://github.com/moneyphp/money). But MoneyPHP uses Currencies everywhere, and thats not applicable for CoreShop. I guess Pimcore had the same Idea, since they are reusing a lot of ideas and code from MoneyPHP.

And, I am not really into the idea of using an object for calculations. So, TLDR: I guess its gonna stay as it is now.

PS: If there is ever gonna be something like a Bridge, the Bridge needs to handle conversion between Pimcore values and CoreShop values. Since the new utility class is only available within the Bundle, CoreShop can't use it anyway. CoreShop does not have a dependency to the framework and therefore won't use stuff from the framework directly.

[Yivan]

Thanks for you explaination, I understand better now.