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.
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 @@
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 @@
*/
@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 @@
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)));
