isotope / core

Core repository of Isotope eCommerce, an eCommerce extension for Contao Open Source CMS
https://isotopeecommerce.org
136 stars 107 forks source link

Rules & Coupons - Precision issues #309

Closed ghost closed 11 years ago

ghost commented 11 years ago

Hello,

because of the binary precision the functions used to create a 2 decimal price for Rules and Coupons (at the end - section "if ($arrRule['applyTo'] == 'cart' && $blnMatch)") is actually creating precision issues.

e.g. if I have a discount of 20% on 144.00, the result would be 28.80 precisely. However, because of this part: (ceil($fltPrice * 100) / 100) it results in 28.79 instead

One possible solution would be to do something like this first: $fltPrice = sscanf(number_format($fltPrice,4), "%f")

That seems to work fine - actually using "5" works too, probably even more as the issue is probably somewhere in the last digit.

--- Originally created by AndreasA on 2011-12-19 14:11:34 (ID 2686)

ghost commented 11 years ago

Of course, you then also need to use $fltPrice = $fltPrice[0]; as sscanf returns an array.

--- Originally created by AndreasA on 2011-12-19 14:17:10

aschempp commented 11 years ago

The mentioned @ceil()@ function does nothing more than round the price. What does @$fltPrice@ contain before it is rounded?

--- Originally created by andreas.schempp on 2011-12-19 14:51:42

ghost commented 11 years ago

I know that it only "rounds" the price. $fltPrice contains exactly 28.80 - according to "die".

Btw. I never said that ceil is the problem The problem probably comes from the multiplication combined with precision errors in the original value.

As said cutting the value of at 4 precision points and do the calculation afterwards works.

Best would be to test it yourself. I used one product with price of 6 Euro and added an amount of 24 to the cart. This combined with a grandtotal discount of "-20%" results in exactly this issue.

Actually I tried a few different values and the result was almost always -0.01 Euro when using a "-20%" discount. In contrast to e.g. "-21%" where it seems to work fine. But, of course, with 21% i never get such neat discount results like 28.80 Euro.

--- Originally created by AndreasA on 2011-12-19 15:54:34

ghost commented 11 years ago

Actually, I have been wondering but many new (object oriented) programming languages do have own data types for money. They are especially there to avoid issues like this one.

The main difference is that those types aren't floating point numbers but like integer with a fixed decimal position and quite a lot of numbers before and after it which is way more accurate than float for such calculations. Well, of course, it also requires more RAM.

Btw. the quickest way to check this issue is by doing the following calculation: echo ceil(0.5500 * 100);

It will result in 56 instead of 55. Actually it might be even more interesting to do this: $val = 0.55 * 100; echo $val."\n"; echo ceil($val);

Because that way you will see that the first echo reports 55 but the second 56.

However, what seems to work is to use the more accurate php calculation function bcmul $val = bcmul(0.55, 100); echo $val."\n"; echo ceil($val);

Results in 55 for both values.

--- Originally created by AndreasA on 2011-12-20 18:15:47

ghost commented 11 years ago

Regarding bcmul, it indeed seems to work fine, if I do something like this: bcdiv(ceil(bcmul($fltPrice, 100)), 100, 2);

--- Originally created by AndreasA on 2011-12-20 18:24:06

ghost commented 11 years ago

Small correction, it would probably need to look something like this (btw. works fine too): bcdiv(ceil(bcmul($fltPrice, 100, 4)), 100, 4)

As otherwise you get values after the decimal point for the first result, which would, of course, result in there to be no reason to use ceil :)

--- Originally created by AndreasA on 2011-12-20 18:28:15

ghost commented 11 years ago

Hmm... I just thought about this but the function is using floor and ceil - one time with negative and one time with positive values, wouldn't be the same if you just used - in both cases: bcdiv(bcmul($fltPrice, 100, 0), 100, 0) ?

Actually, even a simple intval() call or an integer cast should do the trick there, as the functions isn't actually rounding but always cutting off the parts afterwards - btw. is that actually intended? I thought isotope has a setting regarding "rounding" of numbers which seems to be kinda ignored there.

--- Originally created by AndreasA on 2011-12-20 18:45:41

aschempp commented 11 years ago

No feedback for several months. I assume this has been fixed. Otherwise we should start it from scratch.

--- Originally created by andreas.schempp on 2012-04-02 23:40:18