[OFBiz] Dev - rounding bug in X for $Y promotions

[Alex Schmelkin]

We were hit yesterday with a nasty bug in a production site that is running
a "3 of these products for $99.99" promotion and was slammed with traffic.
The right combination of products would result in a total shopping cart
price of  99.99000000000001 (which was displayed to the user as $99.99) --
internally it seems that this type of promotion uses a percentage off each
product in order to compute the final X for $Y deal.

The site uses total price based shipping formulas, 0-49.99:$x, 50-99.99:$y,
100-149.99:$z, and so on.

The code to lookup the shipping options to present to a user for an order
(ShipmentServices.java) looks like this:

if (shippableTotal.doubleValue() >= min && (max == 0 ||
shippableTotal.doubleValue() <= max))
    priceValid = true;

It iterates through all available shipping estimates until it finds matches.

Obviously, 99.99000000000001 will fail that test, no shipping options will
match, and in our case, users were unable to check out.

We will investigate how to make the promotions code handle decimal precision
better when it computes the shopping cart total.  Our quick fix last night
to avoid losing sales was to round the shippableTotal in
calcShipmentCostEstimate:

        int decimalPlace = 2;
        BigDecimal bd = new BigDecimal(shippableTotal.doubleValue());
        bd = bd.setScale(decimalPlace,BigDecimal.ROUND_HALF_UP);
        shippableTotal = new Double(bd.doubleValue());

This turned 99.99000000000001 back into the more sensible 99.99 and checkout
was fixed for people order products running in this promotion.

I'm not sure if this type of fix is desirable in the ofbiz core.  If yes,
I'm happy to submit a patch.

 
_______________________________________________
Dev mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/dev

[Si Chen-2]

Alex,

This is absolutely something that should be submitted and changed.  We
are in the process of updating the existing code to use BigDecimal.

I think the right answer is to
1.  have a centralized arithmetic.properties file to store the
BigDecimal properties, with different properties for different types of
accounting data and
2.  a new helper method for retrieving those properties

I have created an applications/accounting/config/arithmetic.properties
for this purpose, with separate rounding and decimal precisions for
invoices, orders, and customer accounts.  I have also put the helper
methods into org.ofbiz.accounting.util.UtilAccounting.  All of this
should be in the SVN as of r 6138.

I'm going to put the notes in:
http://jira.undersunconsulting.com/browse/OFBIZ-377

Si

Alex Schmelkin wrote:

>We were hit yesterday with a nasty bug in a production site that is running
>a "3 of these products for $99.99" promotion and was slammed with traffic.
>The right combination of products would result in a total shopping cart
>price of  99.99000000000001 (which was displayed to the user as $99.99) --
>internally it seems that this type of promotion uses a percentage off each
>product in order to compute the final X for $Y deal.
>
>The site uses total price based shipping formulas, 0-49.99:$x, 50-99.99:$y,
>100-149.99:$z, and so on.
>
>The code to lookup the shipping options to present to a user for an order
>(ShipmentServices.java) looks like this:
>
>if (shippableTotal.doubleValue() >= min && (max == 0 ||
>shippableTotal.doubleValue() <= max))
>    priceValid = true;
>
>It iterates through all available shipping estimates until it finds matches.
>
>Obviously, 99.99000000000001 will fail that test, no shipping options will
>match, and in our case, users were unable to check out.
>
>We will investigate how to make the promotions code handle decimal precision
>better when it computes the shopping cart total.  Our quick fix last night
>to avoid losing sales was to round the shippableTotal in
>calcShipmentCostEstimate:
>
>        int decimalPlace = 2;
>        BigDecimal bd = new BigDecimal(shippableTotal.doubleValue());
>        bd = bd.setScale(decimalPlace,BigDecimal.ROUND_HALF_UP);
>        shippableTotal = new Double(bd.doubleValue());
>
>This turned 99.99000000000001 back into the more sensible 99.99 and checkout
>was fixed for people order products running in this promotion.
>
>I'm not sure if this type of fix is desirable in the ofbiz core.  If yes,
>I'm happy to submit a patch.
>
>
>_______________________________________________
>Dev mailing list
>[hidden email]
>http://lists.ofbiz.org/mailman/listinfo/dev
>
>  
>

 
_______________________________________________
Dev mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/dev