[commons-numbers] 06/07: NUMBERS-127: Fraction(int, int): Negate numerator and denominator only after reduction to lowest terms

[er...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

erans pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit ac1b27356853d98acfab98db2b9f535325adacf8
Author: Schamschi <he...@gmx.at>
AuthorDate: Mon Jul 1 14:57:58 2019 +0200

    NUMBERS-127: Fraction(int, int): Negate numerator and denominator only after reduction to lowest terms
---
 .../apache/commons/numbers/fraction/Fraction.java  | 24 ++++++++++++++--------
 .../commons/numbers/fraction/CommonTestCases.java  |  3 +++
 2 files changed, 19 insertions(+), 8 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
index d8dbea4..9920d03 100644
--- 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
@@ -162,15 +162,18 @@ public class Fraction
         if (den == 0) {
             throw new ArithmeticException("division by zero");
         }
-        if (den < 0) {
-            if (num == Integer.MIN_VALUE ||
-                den == Integer.MIN_VALUE) {
-                throw new FractionException(FractionException.ERROR_NEGATION_OVERFLOW, num, den);
-            }
-            num = -num;
-            den = -den;
+
+        /*
+         * if num and den are both 2^-31, or if one is 0 and the other is 2^-31,
+         * the calculation of the gcd below will fail. Ensure that this does not
+         * happen by dividing both by 2 in case both are even.
+         */
+        if (((num | den) & 1) == 0) {
+            num >>= 1;
+            den >>= 1;
         }
-        // reduce numerator and denominator by greatest common denominator.
+
+        // reduce numerator and denominator by greatest common divisor.
         final int d = ArithmeticUtils.gcd(num, den);
         if (d > 1) {
             num /= d;
@@ -179,9 +182,14 @@ public class Fraction
 
         // move sign to numerator.
         if (den < 0) {
+            if (num == Integer.MIN_VALUE ||
+                den == Integer.MIN_VALUE) {
+                throw new FractionException(FractionException.ERROR_NEGATION_OVERFLOW, num, den);
+            }
             num = -num;
             den = -den;
         }
+
         this.numerator   = num;
         this.denominator = den;
     }
diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
index 3b71c72..d28ef63 100644
--- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
+++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
@@ -86,6 +86,9 @@ class CommonTestCases {
         testCases.add(new UnaryOperatorTestCase(-2, 4, -1, 2));
         testCases.add(new UnaryOperatorTestCase(2, -4, -1, 2));
 
+        testCases.add(new UnaryOperatorTestCase(2, Integer.MIN_VALUE, -1, - (Integer.MIN_VALUE / 2)));
+        testCases.add(new UnaryOperatorTestCase(Integer.MIN_VALUE, -2, - (Integer.MIN_VALUE / 2), 1));
+
         return testCases;
     }