[NUMBERS-152] add a private constructor for already known reduced num…

… and den, to avoid unneeded gcd.
apache · Aug 26, 2020 · 634ef81 · 634ef81
1 parent c3d173a
commit 634ef81
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 9 deletions.
diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java
@@ -105,6 +105,30 @@ private Fraction(int num, int den) {
         }
     }
 
+    /**
+     * Private constructor: Instances are created using factory methods.
+     *
+     * <p>This constructor should only be invoked when the fraction is known
+     * to be already reduced, and non-ZERO;
+     * otherwise use {@link #Fraction(int, int)}, or {@link #ZERO}.
+     *
+     * @param num Numerator.
+     * @param den Denominator.
+     * @param ignore Ignored, do not really use this, just to make it can overload.
+     */
+    private Fraction(int num, int den, boolean ignore) {
+        if (den == 0) {
+            throw new FractionException(FractionException.ERROR_ZERO_DENOMINATOR);
+        }
+        if (num == den) {
+            numerator = 1;
+            denominator = 1;
+        } else {
+            numerator = num;
+            denominator = den;
+        }
+    }
+
     /**
      * Private constructor: Instances are created using factory methods.
      *
@@ -470,8 +494,8 @@ public Fraction abs() {
     @Override
     public Fraction negate() {
         return numerator == Integer.MIN_VALUE ?
-            new Fraction(numerator, -denominator) :
-            new Fraction(-numerator, denominator);
+            new Fraction(numerator, -denominator, false) :
+            new Fraction(-numerator, denominator, false);
     }
 
     /**
@@ -483,7 +507,7 @@ public Fraction negate() {
      */
     @Override
     public Fraction reciprocal() {
-        return new Fraction(denominator, numerator);
+        return new Fraction(denominator, numerator, false);
     }
 
     /**
@@ -581,7 +605,7 @@ public Fraction subtract(final int value) {
         if (isZero()) {
             // Special case for min value
             return value == Integer.MIN_VALUE ?
-                new Fraction(Integer.MIN_VALUE, -1) :
+                new Fraction(Integer.MIN_VALUE, -1, false) :
                 new Fraction(-value);
         }
         // Convert to numerator with same effective denominator
@@ -674,7 +698,7 @@ public Fraction multiply(final int value) {
         // (see multiply(Fraction) using value / 1 as the argument).
         final int d2 = ArithmeticUtils.gcd(value, denominator);
         return new Fraction(Math.multiplyExact(numerator, value / d2),
-                            denominator / d2);
+                            denominator / d2, false);
     }
 
     /**
@@ -740,7 +764,7 @@ public Fraction divide(final int value) {
         // (see multiply(Fraction) using 1 / value as the argument).
         final int d1 = ArithmeticUtils.gcd(numerator, value);
         return new Fraction(numerator / d1,
-                            Math.multiplyExact(denominator, value / d1));
+                            Math.multiplyExact(denominator, value / d1), false);
     }
 
     /**
@@ -789,18 +813,18 @@ public Fraction pow(final int exponent) {
         }
         if (exponent > 0) {
             return new Fraction(ArithmeticUtils.pow(numerator, exponent),
-                                ArithmeticUtils.pow(denominator, exponent));
+                                ArithmeticUtils.pow(denominator, exponent), false);
         }
         if (exponent == -1) {
             return this.reciprocal();
         }
         if (exponent == Integer.MIN_VALUE) {
             // MIN_VALUE can't be negated
             return new Fraction(ArithmeticUtils.pow(denominator, Integer.MAX_VALUE) * denominator,
-                                ArithmeticUtils.pow(numerator, Integer.MAX_VALUE) * numerator);
+                                ArithmeticUtils.pow(numerator, Integer.MAX_VALUE) * numerator, false);
         }
         return new Fraction(ArithmeticUtils.pow(denominator, -exponent),
-                            ArithmeticUtils.pow(numerator, -exponent));
+                            ArithmeticUtils.pow(numerator, -exponent), false);
     }
 
     /**

diff --git a/...s-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java b/...s-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
@@ -279,6 +279,8 @@ private static List<UnaryOperatorTestCase> collectAbsTestCases() {
         testCases.add(new UnaryOperatorTestCase(10, 21, 10, 21));
         testCases.add(new UnaryOperatorTestCase(-11, 23, 11, 23));
         testCases.add(new UnaryOperatorTestCase(13, -24, -13, -24));
+        testCases.add(new UnaryOperatorTestCase(-10, 10, 1, 1));
+        testCases.add(new UnaryOperatorTestCase(10, -10, 1, 1));
         testCases.add(new UnaryOperatorTestCase(0, 1, 0, 1));
         testCases.add(new UnaryOperatorTestCase(0, -1, 0, 1));