Skip to content

Commit e429d63

Browse files
author
Gilles Sadowski
committed
NUMBERS-207: Incorrect comparison of negative fractions.
1 parent bb0f16e commit e429d63

4 files changed

Lines changed: 12 additions & 2 deletions

File tree

commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1064,7 +1064,9 @@ public int compareTo(final BigFraction other) {
10641064
// Compare absolute magnitude
10651065
final BigInteger nOd = numerator.abs().multiply(other.denominator.abs());
10661066
final BigInteger dOn = denominator.abs().multiply(other.numerator.abs());
1067-
return nOd.compareTo(dOn);
1067+
return lhsSigNum > 0 ?
1068+
nOd.compareTo(dOn) :
1069+
dOn.compareTo(nOd);
10681070
}
10691071

10701072
/**

commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -876,7 +876,9 @@ public int compareTo(Fraction other) {
876876
// Multiplication by the signum is equal to the absolute.
877877
final long nOd = ((long) numerator) * lns * other.denominator * rds;
878878
final long dOn = ((long) denominator) * lds * other.numerator * rns;
879-
return Long.compare(nOd, dOn);
879+
return lhsSigNum > 0 ?
880+
Long.compare(nOd, dOn) :
881+
Long.compare(dOn, nOd);
880882
}
881883

882884
/**

commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ void testCompareTo() {
254254
final BigFraction e = BigFraction.of(1, -2);
255255
final BigFraction f = BigFraction.of(-1, -2);
256256
final BigFraction g = BigFraction.of(-1, Integer.MIN_VALUE);
257+
final BigFraction h = BigFraction.of(-2, 1);
258+
final BigFraction i = BigFraction.of(-1, 1);
257259

258260
Assertions.assertEquals(0, a.compareTo(a));
259261
Assertions.assertEquals(0, a.compareTo(c));
@@ -272,6 +274,7 @@ void testCompareTo() {
272274
Assertions.assertEquals(-1, g.compareTo(f));
273275
Assertions.assertEquals(1, a.compareTo(g));
274276
Assertions.assertEquals(-1, d.compareTo(g));
277+
Assertions.assertEquals(-1, h.compareTo(i)); // JIRA:NUMBERS-207
275278

276279
Assertions.assertEquals(0, BigFraction.of(0, 3).compareTo(BigFraction.of(0, -2)));
277280

commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ void testCompareTo() {
214214
final Fraction e = Fraction.of(1, -2);
215215
final Fraction f = Fraction.of(-1, -2);
216216
final Fraction g = Fraction.of(-1, Integer.MIN_VALUE);
217+
final Fraction h = Fraction.of(-2, 1);
218+
final Fraction i = Fraction.of(-1, 1);
217219

218220
Assertions.assertEquals(0, a.compareTo(a));
219221
Assertions.assertEquals(0, a.compareTo(c));
@@ -232,6 +234,7 @@ void testCompareTo() {
232234
Assertions.assertEquals(-1, g.compareTo(f));
233235
Assertions.assertEquals(1, a.compareTo(g));
234236
Assertions.assertEquals(-1, d.compareTo(g));
237+
Assertions.assertEquals(-1, h.compareTo(i)); // JIRA:NUMBERS-207
235238

236239
Assertions.assertEquals(0, Fraction.of(0, 3).compareTo(Fraction.of(0, -2)));
237240

0 commit comments

Comments
 (0)