Re: svn commit: r1042034 - /ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java

[Scott Gray-2]

Hi Jacques,

The rounding is already present in the divide operation, so the additional setScale wasn't necessary.

Regards
Scott

HotWax Media
http://www.hotwaxmedia.com

On 4/12/2010, at 11:34 AM, [hidden email] wrote:

> Author: jleroux
> Date: Fri Dec  3 22:34:06 2010
> New Revision: 1042034
>
> URL: http://svn.apache.org/viewvc?rev=1042034&view=rev
> Log:
> A suggested change by Jeremy Wickersheimer "Rounding error in prorating returned adjustments" (https://issues.apache.org/jira/browse/OFBIZ-4038) - OFBIZ-4038
>
> The error is in applications/order/src/org/ofbiz/order/order/OrderReturnServices.java the getAdjustmentAmount method is prorating using this code:
>
> newAmount = returnTotal.divide(originalTotal, decimals, rounding).multiply(amount).setScale(decimals, rounding);
>
> it should be:
>
> newAmount = returnTotal.multiply(amount).divide(originalTotal, decimals, rounding);
>
> else by dividing first what should be for example 1/3 of the adjustment ends up being 0.33
>
> JLR: I have added the setScale part
>
> Modified:
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java?rev=1042034&r1=1042033&r2=1042034&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java Fri Dec  3 22:34:06 2010
> @@ -2648,7 +2648,7 @@ public class OrderReturnServices {
>         originalTotal = originalTotal.setScale(decimals, rounding);
>         BigDecimal newAmount = null;
>         if (ZERO.compareTo(originalTotal) != 0) {
> -            newAmount = returnTotal.divide(originalTotal, decimals, rounding).multiply(amount).setScale(decimals, rounding);
> +            newAmount = returnTotal.multiply(amount).divide(originalTotal, decimals, rounding).setScale(decimals, rounding);
>         } else {
>             newAmount = ZERO;
>         }
>
>

[Jacques Le Roux]

Ha yes indeed, thanks Scott. It's fixed

Jacques

Scott Gray wrote:

> Hi Jacques,
>
> The rounding is already present in the divide operation, so the additional setScale wasn't necessary.
>
> Regards
> Scott
>
> HotWax Media
> http://www.hotwaxmedia.com
>
> On 4/12/2010, at 11:34 AM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Fri Dec  3 22:34:06 2010
>> New Revision: 1042034
>>
>> URL: http://svn.apache.org/viewvc?rev=1042034&view=rev
>> Log:
>> A suggested change by Jeremy Wickersheimer "Rounding error in prorating returned adjustments"
>> (https://issues.apache.org/jira/browse/OFBIZ-4038) - OFBIZ-4038
>>
>> The error is in applications/order/src/org/ofbiz/order/order/OrderReturnServices.java the getAdjustmentAmount method is
>> prorating using this code:
>>
>> newAmount = returnTotal.divide(originalTotal, decimals, rounding).multiply(amount).setScale(decimals, rounding);
>>
>> it should be:
>>
>> newAmount = returnTotal.multiply(amount).divide(originalTotal, decimals, rounding);
>>
>> else by dividing first what should be for example 1/3 of the adjustment ends up being 0.33
>>
>> JLR: I have added the setScale part
>>
>> Modified:
>>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java
>>
>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java?rev=1042034&r1=1042033&r2=1042034&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java (original) +++
>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java Fri Dec  3 22:34:06 2010 @@ -2648,7 +2648,7 @@
>>         public class OrderReturnServices { originalTotal = originalTotal.setScale(decimals, rounding);
>>         BigDecimal newAmount = null;
>>         if (ZERO.compareTo(originalTotal) != 0) {
>> -            newAmount = returnTotal.divide(originalTotal, decimals, rounding).multiply(amount).setScale(decimals, rounding);
>> +            newAmount = returnTotal.multiply(amount).divide(originalTotal, decimals, rounding).setScale(decimals, rounding);
>>         } else {
>>             newAmount = ZERO;
>>         }