Repository: incubator-freemarker Updated Branches: refs/heads/2.3-gae 3d2530c08 -> b996adaa2
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-gae 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)); + } + } + } + +}
