commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aherb...@apache.org
Subject [commons-numbers] 03/26: Fixed Fraction.compareTo
Date Thu, 16 Apr 2020 12:18:18 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 36154bba47e78c3a3d1ff9193ffacb89be13c63f
Author: Alex Herbert <aherbert@apache.org>
AuthorDate: Sun Apr 12 21:29:27 2020 +0100

    Fixed Fraction.compareTo
    
    Since the sign can be in the numerator or denominator the signum must be
    checked first before the magnitude can be tested.
---
 .../org/apache/commons/numbers/fraction/BigFraction.java     |  4 +++-
 .../java/org/apache/commons/numbers/fraction/Fraction.java   | 12 ++++++++++--
 .../org/apache/commons/numbers/fraction/FractionTest.java    |  4 ++--
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
index 0d145fb..d293252 100644
--- a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
+++ b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
@@ -992,10 +992,12 @@ public final class BigFraction
         if (lhsSigNum != rhsSigNum) {
             return (lhsSigNum > rhsSigNum) ? 1 : -1;
         }
+        // Same sign.
+        // Avoid a multiply if both fractions are zero
         if (lhsSigNum == 0) {
             return 0;
         }
-
+        // Compare magnitude
         final BigInteger nOd = numerator.multiply(other.denominator);
         final BigInteger dOn = denominator.multiply(other.numerator);
         return nOd.compareTo(dOn);
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 c5a883d..99151f5 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
@@ -696,8 +696,16 @@ public final class Fraction
      */
     @Override
     public int compareTo(Fraction other) {
-        return Long.compare(((long) numerator) * other.denominator,
-                            ((long) denominator) * other.numerator);
+        final int lhsSigNum = signum();
+        final int rhsSigNum = other.signum();
+
+        if (lhsSigNum != rhsSigNum) {
+            return (lhsSigNum > rhsSigNum) ? 1 : -1;
+        }
+        // Same sign: compare magnitude
+        final long nOd = ((long) numerator) * other.denominator;
+        final long dOn = ((long) denominator) * other.numerator;
+        return Long.compare(nOd, dOn);
     }
 
     /**
diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
index d1916f6..584c28f 100644
--- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
+++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
@@ -153,8 +153,8 @@ public class FractionTest {
         Assertions.assertEquals(-1, b.compareTo(a));
         Assertions.assertEquals(-1, d.compareTo(a));
         Assertions.assertEquals(1, a.compareTo(d));
-//        Assertions.assertEquals(-1, e.compareTo(a));
-//        Assertions.assertEquals(1, a.compareTo(e));
+        Assertions.assertEquals(-1, e.compareTo(a));
+        Assertions.assertEquals(1, a.compareTo(e));
         Assertions.assertEquals(0, d.compareTo(e));
 
         Assertions.assertEquals(0, Fraction.of(0, 3).compareTo(Fraction.of(0, -2)));


Mime
View raw message