Improvements to Rational handling

drewnoakes · May 5, 2022 · 34092f5 · 34092f5
1 parent e8ba160
commit 34092f5
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 12 deletions.
diff --git a/Source/com/drew/lang/Rational.java b/Source/com/drew/lang/Rational.java
@@ -217,10 +217,6 @@ public String toSimpleString(boolean allowDecimal)
             return toString();
         } else if (isInteger()) {
             return Integer.toString(intValue());
-        } else if (_numerator != 1 && _denominator % _numerator == 0) {
-            // common factor between denominator and numerator
-            long newDenominator = _denominator / _numerator;
-            return new Rational(1, newDenominator).toSimpleString(allowDecimal);
         } else {
             Rational simplifiedInstance = getSimplifiedInstance();
             if (allowDecimal) {
@@ -314,9 +310,17 @@ public int hashCode()
     @NotNull
     public Rational getSimplifiedInstance()
     {
-        long gcd = GCD(_numerator, _denominator);
+        long n = _numerator;
+        long d = _denominator;
 
-        return new Rational(_numerator / gcd, _denominator / gcd);
+        if (d < 0) {
+            n = -n;
+            d = -d;
+        }
+
+        long gcd = GCD(n, d);
+
+        return new Rational(n / gcd, d / gcd);
     }
 
     private static long GCD(long a, long b)

diff --git a/Tests/com/drew/lang/RationalTest.java b/Tests/com/drew/lang/RationalTest.java
@@ -72,6 +72,10 @@ public void testToSimpleString() throws Exception
         Rational twoThirds = new Rational(10, 15);
         assertEquals("2/3", twoThirds.toSimpleString(true));
 
+        Rational twoSixths = new Rational(2, 6);
+        assertEquals("1/3", twoSixths.toSimpleString(true));
+        assertEquals("1/3", twoSixths.toSimpleString(false));
+
         Rational two = new Rational(10, 5);
         assertTrue(two.isInteger());
         assertEquals("2", two.toSimpleString(true));
@@ -93,10 +97,6 @@ public void testToSimpleString() throws Exception
         assertTrue(zero.isInteger());
         assertEquals("0", zero.toSimpleString(true));
         assertEquals("0", zero.toSimpleString(false));
-
-        // not sure this is a nice presentation of rationals.  won't implement it for now.
-//        Rational twoAndAHalf = new Rational(10,4);
-//        assertEquals("2 1/2", twoAndAHalf.toSimpleString());
     }
 
     @Test
@@ -215,7 +215,7 @@ public void simplifiedInstances()
             assertEquals(actualSimple.doubleValue(), complex.doubleValue(), 0.0001);
         }
 
-        simple = new Rational(1, -2);
+        simple = new Rational(-1, 2);
 
         for (int prime : _primes)
         {
@@ -226,7 +226,7 @@ public void simplifiedInstances()
             assertEquals(actualSimple.doubleValue(), complex.doubleValue(), 0.0001);
         }
 
-        simple = new Rational(-1, -2);
+        simple = new Rational(1, 2);
 
         for (int prime : _primes)
         {
@@ -241,4 +241,25 @@ public void simplifiedInstances()
         assertEquals(new Rational(-32768, 32767), new Rational(-32768, 32767).getSimplifiedInstance());
     }
 
+    @Test
+    public void getSimplifiedInstance_FlipsSignsIfNeeded()
+    {
+        Rational r = new Rational(1, -2);
+
+        Rational s = r.getSimplifiedInstance();
+
+        assertEquals(-1, s.getNumerator());
+        assertEquals(2, s.getDenominator());
+    }
+
+    @Test
+    public void getSimplifiedInstance_RemovesSignsIfNeeded()
+    {
+        Rational r = new Rational(-1, -2);
+
+        Rational s = r.getSimplifiedInstance();
+
+        assertEquals(1, s.getNumerator());
+        assertEquals(2, s.getDenominator());
+    }
 }