The default arithmetic engine (ArithmeticEngine.BIGDECIMAL_ENGINE) can now 
compare infinite (both positive and negative) to any other standard type. 
Earlier, since BigDecimal can't represent infinite, it was only working in 
certain special cases. Also did some performance optimizations to slightly 
decrease the impact and number of conversions to BigDecimal.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/b996adaa
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/b996adaa
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/b996adaa

Branch: refs/heads/2.3
Commit: b996adaa2a574b269c2a83fd7a2a7522aea62e3c
Parents: 3d2530c
Author: ddekany <ddek...@apache.org>
Authored: Sun Mar 11 22:23:08 2018 +0100
Committer: ddekany <ddek...@apache.org>
Committed: Sun Mar 11 22:23:08 2018 +0100

----------------------------------------------------------------------
 .../java/freemarker/core/ArithmeticEngine.java  | 110 +++++++++++++++++--
 .../freemarker/template/utility/NumberUtil.java |  13 ++-
 src/manual/en_US/book.xml                       |  11 ++
 .../core/BigDecimalArithmeticEngineTest.java    |  97 ++++++++++++++++
 4 files changed, 221 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/b996adaa/src/main/java/freemarker/core/ArithmeticEngine.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/ArithmeticEngine.java 
b/src/main/java/freemarker/core/ArithmeticEngine.java
index 9eedf64..31648f9 100644
--- a/src/main/java/freemarker/core/ArithmeticEngine.java
+++ b/src/main/java/freemarker/core/ArithmeticEngine.java
@@ -111,9 +111,8 @@ public abstract class ArithmeticEngine {
      * number it receives into {@link BigDecimal}, then operates on these
      * converted {@link BigDecimal}s.
      */
-    public static class BigDecimalEngine
-    extends
-        ArithmeticEngine {
+    public static class BigDecimalEngine extends ArithmeticEngine {
+        
         @Override
         public int compareNumbers(Number first, Number second) {
             // We try to find the result based on the sign (+/-/0) first, 
because:
@@ -126,9 +125,83 @@ public abstract class ArithmeticEngine {
             } else if (firstSignum == 0 && secondSignum == 0) {
                 return 0;
             } else {
-                BigDecimal left = toBigDecimal(first);
-                BigDecimal right = toBigDecimal(second);
-                return left.compareTo(right);
+                // The most common case is comparing values of the same type. 
As BigDecimal can represent all of these
+                // with loseless round-trip (i.e., converting to BigDecimal 
and then back the original type gives the
+                // original value), we can avoid conversion to BigDecimal 
without changing the result.
+                if (first.getClass() == second.getClass()) {
+                    // Bit of optimization for this is a very common case:
+                    if (first instanceof BigDecimal) {
+                        return ((BigDecimal) first).compareTo((BigDecimal) 
second);
+                    }
+                    
+                    if (first instanceof Integer) {
+                        return ((Integer) first).compareTo((Integer) second);
+                    }
+                    if (first instanceof Long) {
+                        return ((Long) first).compareTo((Long) second);
+                    }
+                    if (first instanceof Double) {
+                        return ((Double) first).compareTo((Double) second);
+                    }
+                    if (first instanceof Float) {
+                        return ((Float) first).compareTo((Float) second);
+                    }
+                    if (first instanceof Byte) {
+                        return ((Byte) first).compareTo((Byte) second);
+                    }
+                    if (first instanceof Short) {
+                        return ((Short) first).compareTo((Short) second);
+                    }
+                }
+                // We are going to compare values of two different types.
+                
+                // Handle infinity before we try conversion to BigDecimal, as 
that BigDecimal can't represent that:
+                if (first instanceof Double) {
+                    double firstD = first.doubleValue();
+                    if (Double.isInfinite(firstD)) {
+                        if 
(NumberUtil.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(second)) {
+                            return  firstD == Double.NEGATIVE_INFINITY ? -1 : 
1;
+                        }
+                        if (second instanceof Float) {
+                            return Double.compare(firstD, 
second.doubleValue());
+                        }
+                    }
+                }
+                if (first instanceof Float) {
+                    float firstF = first.floatValue();
+                    if (Float.isInfinite(firstF)) {
+                        if 
(NumberUtil.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(second)) {
+                            return firstF == Float.NEGATIVE_INFINITY ? -1 : 1;
+                        }
+                        if (second instanceof Double) {
+                            return Double.compare(firstF, 
second.doubleValue());
+                        }
+                    }
+                }
+                if (second instanceof Double) {
+                    double secondD = second.doubleValue();
+                    if (Double.isInfinite(secondD)) {
+                        if 
(NumberUtil.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(first)) {
+                            return secondD == Double.NEGATIVE_INFINITY ? 1 : 
-1;
+                        }
+                        if (first instanceof Float) {
+                            return Double.compare(first.doubleValue(), 
secondD);
+                        }
+                    }
+                }
+                if (second instanceof Float) {
+                    float secondF = second.floatValue();
+                    if (Float.isInfinite(secondF)) {
+                        if 
(NumberUtil.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(first)) {
+                            return secondF == Float.NEGATIVE_INFINITY ? 1 : -1;
+                        }
+                        if (first instanceof Double) {
+                            return Double.compare(first.doubleValue(), 
secondF);
+                        }
+                    }
+                }
+                
+                return toBigDecimal(first).compareTo(toBigDecimal(second));
             }
         }
     
@@ -526,10 +599,33 @@ public abstract class ArithmeticEngine {
         }
     }
 
+    /**
+     * Convert a {@code Number} to {@link BigDecimal}.
+     * 
+     * @throws NumberFormatException
+     *             If the conversion is not possible, e.g. Infinite and NaN 
can't be converted to {@link BigDecimal}.
+     */
     private static BigDecimal toBigDecimal(Number num) {
+        if (num instanceof BigDecimal) {
+            return (BigDecimal) num;
+        }
+        if (num instanceof Integer || num instanceof Long || num instanceof 
Byte || num instanceof Short) {
+            return BigDecimal.valueOf(num.longValue());
+        }
+        if (num instanceof BigInteger) {
+            return new BigDecimal((BigInteger) num);
+        }
         try {
-            return num instanceof BigDecimal ? (BigDecimal) num : new 
BigDecimal(num.toString());
+            // Why toString? It's partly for backward compatibility. But it's 
also better for Double (and Float) values
+            // than new BigDecimal(someDouble), which is overly precise. For 
example, if you have `double d = 0.1`, then
+            // `new BigDecimal(d).equals(new BigDecimal("0.1"))` is `false`, 
while
+            // `new BigDecimal(Double.toString(d)).equals(new 
BigDecimal("0.1"))` is `true`.
+            return new BigDecimal(num.toString());
         } catch (NumberFormatException e) {
+            if (NumberUtil.isInfinite(num)) {
+                throw new NumberFormatException("It's impossible to convert an 
infinte value ("
+                        + num.getClass().getSimpleName() + " " + num + ") to 
BigDecimal.");
+            }
             // The exception message is useless, so we add a new one:
             throw new NumberFormatException("Can't parse this as BigDecimal 
number: " + StringUtil.jQuote(num));
         }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/b996adaa/src/main/java/freemarker/template/utility/NumberUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/utility/NumberUtil.java 
b/src/main/java/freemarker/template/utility/NumberUtil.java
index a22deff..9cd3382 100644
--- a/src/main/java/freemarker/template/utility/NumberUtil.java
+++ b/src/main/java/freemarker/template/utility/NumberUtil.java
@@ -41,7 +41,7 @@ public class NumberUtil {
             return ((Double) num).isInfinite();
         } else if (num instanceof Float) {
             return ((Float) num).isInfinite();
-        } else if (isNonFPNumberOfSupportedClass(num)) {
+        } else if (hasTypeThatIsKnownToNotSupportInfiniteAndNaN(num)) {
             return false;
         } else {
             throw new UnsupportedNumberClassException(num.getClass());
@@ -53,7 +53,7 @@ public class NumberUtil {
             return ((Double) num).isNaN();
         } else if (num instanceof Float) {
             return ((Float) num).isNaN();
-        } else if (isNonFPNumberOfSupportedClass(num)) {
+        } else if (hasTypeThatIsKnownToNotSupportInfiniteAndNaN(num)) {
             return false;
         } else {
             throw new UnsupportedNumberClassException(num.getClass());
@@ -114,7 +114,14 @@ public class NumberUtil {
         // condition, but stripTrailingZeros was slower than setScale + 
compareTo.
     }
     
-    private static boolean isNonFPNumberOfSupportedClass(Number num) {
+    /**
+     * Tells if the type of the parameter number is known to not be able to 
represent infinite (positive or negative)
+     * and NaN. If this returns {@code false}, that doesn't mean that it can 
do that, because it's maybe just that this
+     * utility doesn't know that type.
+     * 
+     * @since 2.3.28
+     */
+    public static boolean hasTypeThatIsKnownToNotSupportInfiniteAndNaN(Number 
num) {
         return num instanceof Integer || num instanceof BigDecimal || num 
instanceof Long
                 || num instanceof Short || num instanceof Byte || num 
instanceof BigInteger;
     }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/b996adaa/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index e4cf87a..db1d1f3 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -27468,6 +27468,17 @@ TemplateModel x = env.getVariable("x");  // get 
variable x</programlisting>
             </listitem>
 
             <listitem>
+              <para>The default arithmetic engine
+              (<literal>ArithmeticEngine.BIGDECIMAL_ENGINE</literal>) can now
+              compare infinite (both positive and negative) to any other
+              standard type. Earlier, since <literal>BigDecimal</literal>
+              can't represent infinite, it was only working in certain special
+              cases. Also did some performance optimizations to slightly
+              decrease the impact and number of conversions to
+              <literal>BigDecimal</literal>.</para>
+            </listitem>
+
+            <listitem>
               <para>Added
               
<literal>TemplateModelUtils.getKeyValuePairIterator(TemplateHashModelEx)</literal>
               static utility class, which can be used to get a

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/b996adaa/src/test/java/freemarker/core/BigDecimalArithmeticEngineTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/core/BigDecimalArithmeticEngineTest.java 
b/src/test/java/freemarker/core/BigDecimalArithmeticEngineTest.java
new file mode 100644
index 0000000..d6db2f7
--- /dev/null
+++ b/src/test/java/freemarker/core/BigDecimalArithmeticEngineTest.java
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package freemarker.core;
+
+import static freemarker.core.ArithmeticEngine.*;
+import static org.junit.Assert.*;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+import org.junit.Test;
+
+public class BigDecimalArithmeticEngineTest {
+
+    @Test
+    public void compareNumbersZeroTest() {
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(0, new 
BigDecimal("-0")));
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(0.0, new 
BigDecimal("-0")));
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(-0.0, new 
BigDecimal("+0")));
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(-0.0, 0.0));
+    }
+
+    @Test
+    public void compareNumbersNoRoundingGlitchTest() {
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(1.1, new 
BigDecimal("1.1")));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(Double.MIN_VALUE, 
Math.nextUp(Double.MIN_VALUE)));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(
+                Double.MIN_VALUE, new BigDecimal("" + 
Math.nextUp(Double.MIN_VALUE))));
+    }
+
+    @Test
+    public void compareNumbersSameTypeTest() {
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(1, 2));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(1L, 2L));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers((short) 1, (short) 
2));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers((byte) 1, (byte) 2));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(1.0, 2.0));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(1.0f, 2.0f));
+        assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(BigDecimal.ONE, 
BigDecimal.TEN));
+    }
+
+    @Test
+    public void compareNumbersScaleDoesNotMatterTest() {
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(1.0, new 
BigDecimal("1")));
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(1.0, new 
BigDecimal("1.00")));
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(1.0f, new 
BigDecimal("0.1E1")));
+        assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(1, new 
BigDecimal("1.0")));
+    }
+
+    @Test
+    public void compareNumbersInfinityTest() {
+        for (boolean isFloat : new boolean[] { false, true }) {
+            Number negInf = isFloat ? Float.NEGATIVE_INFINITY : 
Double.NEGATIVE_INFINITY;
+            Number posInf = isFloat ? Float.POSITIVE_INFINITY : 
Double.POSITIVE_INFINITY;
+            
+            assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(negInf, posInf));
+            assertEquals(1, BIGDECIMAL_ENGINE.compareNumbers(posInf, negInf));
+            assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(posInf, posInf));
+            assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(negInf, negInf));
+            
+            Number otherNegInf = !isFloat ? Float.NEGATIVE_INFINITY : 
Double.NEGATIVE_INFINITY;
+            Number otherPosInf = !isFloat ? Float.POSITIVE_INFINITY : 
Double.POSITIVE_INFINITY;
+            assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(negInf, 
otherPosInf));
+            assertEquals(1, BIGDECIMAL_ENGINE.compareNumbers(posInf, 
otherNegInf));
+            assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(posInf, 
otherPosInf));
+            assertEquals(0, BIGDECIMAL_ENGINE.compareNumbers(negInf, 
otherNegInf));
+            
+            for (Number one : new Number[] {
+                    BigDecimal.ONE, 1.0, 1f, 1, 1L, (byte) 1, (short) 1, 
BigInteger.ONE,
+                    BigDecimal.ONE.negate(), -1.0, -1f, -1, -1L, (byte) -1, 
(short) -1, BigInteger.ONE.negate(),
+                    BigDecimal.ZERO, 0.0, 0f, 0, 0L, (byte) 0, (short) 0, 
BigInteger.ZERO }) {
+                assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(negInf, 
one));
+                assertEquals(1, BIGDECIMAL_ENGINE.compareNumbers(posInf, one));
+                assertEquals(1, BIGDECIMAL_ENGINE.compareNumbers(one, negInf));
+                assertEquals(-1, BIGDECIMAL_ENGINE.compareNumbers(one, 
posInf));
+            }
+        }
+    }
+
+}

Reply via email to