(commons-numbers) branch master updated: NUMBERS-207: Incorrect comparison of negative fractions.

[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


The following commit(s) were added to refs/heads/master by this push:
     new e429d637 NUMBERS-207: Incorrect comparison of negative fractions.
e429d637 is described below

commit e429d6378ca4070623c3abe147b96d0e7e577f70
Author: Gilles Sadowski <gi...@gmail.com>
AuthorDate: Sat Mar 15 19:58:25 2025 +0100

    NUMBERS-207: Incorrect comparison of negative fractions.
---
 .../main/java/org/apache/commons/numbers/fraction/BigFraction.java    | 4 +++-
 .../src/main/java/org/apache/commons/numbers/fraction/Fraction.java   | 4 +++-
 .../java/org/apache/commons/numbers/fraction/BigFractionTest.java     | 3 +++
 .../test/java/org/apache/commons/numbers/fraction/FractionTest.java   | 3 +++
 4 files changed, 12 insertions(+), 2 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 5796b76f..0a34b7dc 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
@@ -1064,7 +1064,9 @@ public final class BigFraction
         // Compare absolute magnitude
         final BigInteger nOd = numerator.abs().multiply(other.denominator.abs());
         final BigInteger dOn = denominator.abs().multiply(other.numerator.abs());
-        return nOd.compareTo(dOn);
+        return lhsSigNum > 0 ?
+            nOd.compareTo(dOn) :
+            dOn.compareTo(nOd);
     }
 
     /**
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 07c384f9..4f89c1eb 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
@@ -876,7 +876,9 @@ public final class Fraction
         // Multiplication by the signum is equal to the absolute.
         final long nOd = ((long) numerator) * lns * other.denominator * rds;
         final long dOn = ((long) denominator) * lds * other.numerator * rns;
-        return Long.compare(nOd, dOn);
+        return lhsSigNum > 0 ?
+            Long.compare(nOd, dOn) :
+            Long.compare(dOn, nOd);
     }
 
     /**
diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
index c1095d22..ae6a6d8a 100644
--- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
+++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
@@ -254,6 +254,8 @@ class BigFractionTest {
         final BigFraction e = BigFraction.of(1, -2);
         final BigFraction f = BigFraction.of(-1, -2);
         final BigFraction g = BigFraction.of(-1, Integer.MIN_VALUE);
+        final BigFraction h = BigFraction.of(-2, 1);
+        final BigFraction i = BigFraction.of(-1, 1);
 
         Assertions.assertEquals(0, a.compareTo(a));
         Assertions.assertEquals(0, a.compareTo(c));
@@ -272,6 +274,7 @@ class BigFractionTest {
         Assertions.assertEquals(-1, g.compareTo(f));
         Assertions.assertEquals(1, a.compareTo(g));
         Assertions.assertEquals(-1, d.compareTo(g));
+        Assertions.assertEquals(-1, h.compareTo(i)); // JIRA:NUMBERS-207
 
         Assertions.assertEquals(0, BigFraction.of(0, 3).compareTo(BigFraction.of(0, -2)));
 
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 def672ff..fbc0b514 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
@@ -214,6 +214,8 @@ class FractionTest {
         final Fraction e = Fraction.of(1, -2);
         final Fraction f = Fraction.of(-1, -2);
         final Fraction g = Fraction.of(-1, Integer.MIN_VALUE);
+        final Fraction h = Fraction.of(-2, 1);
+        final Fraction i = Fraction.of(-1, 1);
 
         Assertions.assertEquals(0, a.compareTo(a));
         Assertions.assertEquals(0, a.compareTo(c));
@@ -232,6 +234,7 @@ class FractionTest {
         Assertions.assertEquals(-1, g.compareTo(f));
         Assertions.assertEquals(1, a.compareTo(g));
         Assertions.assertEquals(-1, d.compareTo(g));
+        Assertions.assertEquals(-1, h.compareTo(i)); // JIRA:NUMBERS-207
 
         Assertions.assertEquals(0, Fraction.of(0, 3).compareTo(Fraction.of(0, -2)));