svn commit: r1786079 - /ofbiz/ofbiz-framework/trunk/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java

Author: jleroux
Date: Wed Mar  8 21:27:30 2017
New Revision: 1786079

URL: http://svn.apache.org/viewvc?rev=1786079&view=rev
Log:
Fixed: compareBigDecimals in org.ofbiz.minilang.method.conditional.Compare
does not compare certain values correctly
(OFBIZ-6386)

compareBigDecimals scales down and rounds up meaning you lose information and
the comparison result is not as expected

Thanks: Gareth Carter for the patch, Mridul Pathak and Jacopo for discussing the
topic in all details (see the Jira issue)

Modified:
    ofbiz/ofbiz-framework/trunk/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java

Modified: ofbiz/ofbiz-framework/trunk/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java?rev=1786079&r1=1786078&r2=1786079&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java Wed Mar  8 21:27:30 2017
@@ -18,8 +18,6 @@
  *******************************************************************************/
 package org.apache.ofbiz.minilang.method.conditional;
 
-import java.math.BigDecimal;
-import java.math.RoundingMode;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -49,18 +47,6 @@ public abstract class Compare {
         }
     }
 
-    private static int compareBigDecimals(BigDecimal lBigDecimal, BigDecimal rBigDecimal) {
-        // Developers: Do not change this. We are comparing two fixed-point decimal numbers,
-        // not performing accounting calculations - so it is okay to specify the rounding mode.
-        int decimals = lBigDecimal.scale();
-        if (rBigDecimal.scale() < decimals) {
-            lBigDecimal = lBigDecimal.setScale(rBigDecimal.scale(), RoundingMode.UP);
-        } else if (decimals < rBigDecimal.scale()) {
-            rBigDecimal = rBigDecimal.setScale(decimals, RoundingMode.UP);
-        }
-        return lBigDecimal.compareTo(rBigDecimal);
-    }
-
     private static Map<String, Compare> createInstanceMap() {
         Map<String, Compare> writableMap = new HashMap<String, Compare>(10);
         writableMap.put("contains", new CompareContains());
@@ -132,12 +118,6 @@ public abstract class Compare {
             if (convertedRvalue == null) {
                 return false;
             }
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) == 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -154,12 +134,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) > 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -176,12 +150,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) >= 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -225,12 +193,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) < 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -247,12 +209,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) <= 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -274,12 +230,6 @@ public abstract class Compare {
             if (convertedRvalue == null) {
                 return true;
             }
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) != 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);