You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by er...@apache.org on 2019/10/22 13:45:32 UTC
[commons-numbers] 02/05: NUMBERS-99: Remove requirement that the
sign be held in the numerator.
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
commit 44ea3e26fa7610f3635aa225d24c4573ad5e54f2
Author: Gilles Sadowski <gi...@harfang.homelinux.org>
AuthorDate: Tue Oct 22 10:59:10 2019 +0200
NUMBERS-99: Remove requirement that the sign be held in the numerator.
New "signum" method.
Unit tests updated.
---
.../commons/numbers/fraction/BigFraction.java | 86 +++++++++-------------
.../apache/commons/numbers/fraction/Fraction.java | 82 +++++++++++----------
.../commons/numbers/fraction/BigFractionTest.java | 27 ++-----
.../commons/numbers/fraction/CommonTestCases.java | 21 ++++--
.../commons/numbers/fraction/FractionTest.java | 24 ++----
5 files changed, 105 insertions(+), 135 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 ae1e284..c5c3fc3 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
@@ -26,7 +26,10 @@ import org.apache.commons.numbers.core.ArithmeticUtils;
* Representation of a rational number without any overflow. This class is
* immutable.
*/
-public class BigFraction extends Number implements Comparable<BigFraction>, Serializable {
+public class BigFraction
+ extends Number
+ implements Comparable<BigFraction>,
+ Serializable {
/** A fraction representing "0". */
public static final BigFraction ZERO = of(0);
@@ -64,28 +67,17 @@ public class BigFraction extends Number implements Comparable<BigFraction>, Seri
if (den.signum() == 0) {
throw new FractionException(FractionException.ERROR_ZERO_DENOMINATOR);
}
- if (num.signum() == 0) {
- numerator = BigInteger.ZERO;
- denominator = BigInteger.ONE;
- } else {
-
- // reduce numerator and denominator by greatest common denominator
- final BigInteger gcd = num.gcd(den);
- if (BigInteger.ONE.compareTo(gcd) < 0) {
- num = num.divide(gcd);
- den = den.divide(gcd);
- }
- // move sign to numerator
- if (den.signum() == -1) {
- num = num.negate();
- den = den.negate();
- }
-
- // store the values in the final fields
- numerator = num;
- denominator = den;
+ // reduce numerator and denominator by greatest common denominator
+ final BigInteger gcd = num.gcd(den);
+ if (BigInteger.ONE.compareTo(gcd) < 0) {
+ num = num.divide(gcd);
+ den = den.divide(gcd);
}
+
+ // store the values in the final fields
+ numerator = num;
+ denominator = den;
}
/**
@@ -387,41 +379,15 @@ public class BigFraction extends Number implements Comparable<BigFraction>, Seri
/**
* <p>
- * Creates a <code>BigFraction</code> instance with the 2 parts of a fraction
- * Y/Z.
- * </p>
- *
- * <p>
- * Any negative signs are resolved to be on the numerator.
- * </p>
- *
- * @param numerator
- * the numerator, for example the three in 'three sevenths'.
- * @param denominator
- * the denominator, for example the seven in 'three sevenths'.
- * @return a new fraction instance, with the numerator and denominator
- * reduced.
- * @throws ArithmeticException
- * if the denominator is <code>zero</code>.
- */
- public static BigFraction getReducedFraction(final int numerator,
- final int denominator) {
- if (numerator == 0) {
- return ZERO; // normalize zero.
- }
-
- return of(numerator, denominator);
- }
-
- /**
- * <p>
* Returns the absolute value of this {@link BigFraction}.
* </p>
*
* @return the absolute value as a {@link BigFraction}.
*/
public BigFraction abs() {
- return (numerator.signum() == 1) ? this : negate();
+ return signum() >= 0 ?
+ this :
+ negate();
}
/**
@@ -1090,6 +1056,26 @@ public class BigFraction extends Number implements Comparable<BigFraction>, Seri
}
/**
+ * Retrieves the sign of this fraction.
+ *
+ * @return -1 if the value is strictly negative, 1 if it is strictly
+ * positive, 0 if it is 0.
+ */
+ public int signum() {
+ final int numS = numerator.signum();
+ final int denS = denominator.signum();
+
+ if ((numS > 0 && denS > 0) ||
+ (numS < 0 && denS < 0)) {
+ return 1;
+ } else if (numS == 0) {
+ return 0;
+ } else {
+ return -1;
+ }
+ }
+
+ /**
* <p>
* Return the additive inverse of this fraction, returning the result in
* reduced form.
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 68a2ca5..920dc62 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
@@ -163,33 +163,28 @@ public class Fraction
throw new ArithmeticException("division by zero");
}
- // If num and den are both 2^-31, or if one is 0 and the other is 2^-31,
- // the calculation of the gcd below will fail. Ensure that this does not
- // happen by dividing both by 2 in case both are even.
- if (((num | den) & 1) == 0) {
- num >>= 1;
- den >>= 1;
- }
-
- // Reduce numerator and denominator by greatest common divisor.
- final int d = ArithmeticUtils.gcd(num, den);
- if (d > 1) {
- num /= d;
- den /= d;
- }
+ if (num == den) {
+ numerator = 1;
+ denominator = 1;
+ } else {
+ // If num and den are both 2^-31, or if one is 0 and the other is 2^-31,
+ // the calculation of the gcd below will fail. Ensure that this does not
+ // happen by dividing both by 2 in case both are even.
+ if (((num | den) & 1) == 0) {
+ num >>= 1;
+ den >>= 1;
+ }
- // Move sign to numerator.
- if (den < 0) {
- if (num == Integer.MIN_VALUE ||
- den == Integer.MIN_VALUE) {
- throw new FractionException(FractionException.ERROR_NEGATION_OVERFLOW, num, den);
+ // Reduce numerator and denominator by greatest common divisor.
+ final int d = ArithmeticUtils.gcd(num, den);
+ if (d > 1) {
+ num /= d;
+ den /= d;
}
- num = -num;
- den = -den;
- }
- this.numerator = num;
- this.denominator = den;
+ numerator = num;
+ denominator = den;
+ }
}
/**
@@ -277,13 +272,9 @@ public class Fraction
* @return the absolute value.
*/
public Fraction abs() {
- Fraction ret;
- if (numerator >= 0) {
- ret = this;
- } else {
- ret = negate();
- }
- return ret;
+ return signum() >= 0 ?
+ this :
+ negate();
}
/**
@@ -346,7 +337,7 @@ public class Fraction
*/
@Override
public float floatValue() {
- return (float)doubleValue();
+ return (float) doubleValue();
}
/**
@@ -392,18 +383,31 @@ public class Fraction
}
/**
+ * Retrieves the sign of this fraction.
+ *
+ * @return -1 if the value is strictly negative, 1 if it is strictly
+ * positive, 0 if it is 0.
+ */
+ public int signum() {
+ if ((numerator > 0 && denominator > 0) ||
+ (numerator < 0 && denominator < 0)) {
+ return 1;
+ } else if (numerator == 0) {
+ return 0;
+ } else {
+ return -1;
+ }
+ }
+
+ /**
* Computes the additive inverse of this fraction.
*
* @return the opposite.
*/
public Fraction negate() {
- if (numerator == Integer.MIN_VALUE) {
- throw new FractionException(FractionException.ERROR_NEGATION_OVERFLOW,
- numerator,
- denominator);
- }
-
- return new Fraction(-numerator, denominator);
+ return numerator == Integer.MIN_VALUE ?
+ new Fraction(numerator, -denominator) :
+ new Fraction(-numerator, denominator);
}
/**
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 c36aa15..162d567 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
@@ -488,18 +488,18 @@ public class BigFractionTest {
f1 = BigFraction.of(Integer.MIN_VALUE, Integer.MAX_VALUE);
f = f1.divide(BigInteger.valueOf(Integer.MIN_VALUE));
- Assertions.assertEquals(Integer.MAX_VALUE, f.getDenominatorAsInt());
- Assertions.assertEquals(1, f.getNumeratorAsInt());
+ Assertions.assertEquals(-Integer.MAX_VALUE, f.getDenominatorAsInt());
+ Assertions.assertEquals(-1, f.getNumeratorAsInt());
f1 = BigFraction.of(Integer.MIN_VALUE, Integer.MAX_VALUE);
f = f1.divide(Integer.MIN_VALUE);
- Assertions.assertEquals(Integer.MAX_VALUE, f.getDenominatorAsInt());
- Assertions.assertEquals(1, f.getNumeratorAsInt());
+ Assertions.assertEquals(-Integer.MAX_VALUE, f.getDenominatorAsInt());
+ Assertions.assertEquals(-1, f.getNumeratorAsInt());
f1 = BigFraction.of(Integer.MIN_VALUE, Integer.MAX_VALUE);
f = f1.divide((long) Integer.MIN_VALUE);
- Assertions.assertEquals(Integer.MAX_VALUE, f.getDenominatorAsInt());
- Assertions.assertEquals(1, f.getNumeratorAsInt());
+ Assertions.assertEquals(-Integer.MAX_VALUE, f.getDenominatorAsInt());
+ Assertions.assertEquals(-1, f.getNumeratorAsInt());
}
@@ -568,21 +568,6 @@ public class BigFractionTest {
}
@Test
- public void testGetReducedFraction() {
- BigFraction threeFourths = BigFraction.of(3, 4);
- Assertions.assertEquals(threeFourths, BigFraction.getReducedFraction(6, 8));
- Assertions.assertEquals(BigFraction.ZERO, BigFraction.getReducedFraction(0, -1));
- try {
- BigFraction.getReducedFraction(1, 0);
- Assertions.fail("expecting ArithmeticException");
- } catch (ArithmeticException ex) {
- // expected
- }
- Assertions.assertEquals(-1, BigFraction.getReducedFraction(2, Integer.MIN_VALUE).getNumeratorAsInt());
- Assertions.assertEquals(-1, BigFraction.getReducedFraction(1, -1).getNumeratorAsInt());
- }
-
- @Test
public void testPow() {
Assertions.assertEquals(BigFraction.of(8192, 1594323), BigFraction.of(2, 3).pow(13));
Assertions.assertEquals(BigFraction.of(8192, 1594323), BigFraction.of(2, 3).pow(13l));
diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
index 805de0c..6753282 100644
--- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
+++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/CommonTestCases.java
@@ -78,16 +78,16 @@ class CommonTestCases {
testCases.add(new UnaryOperatorTestCase(0, 1, 0, 1));
testCases.add(new UnaryOperatorTestCase(0, 2, 0, 1));
- testCases.add(new UnaryOperatorTestCase(0, -1, 0, 1));
+ testCases.add(new UnaryOperatorTestCase(0, -1, 0, -1));
testCases.add(new UnaryOperatorTestCase(1, 2, 1, 2));
testCases.add(new UnaryOperatorTestCase(2, 4, 1, 2));
testCases.add(new UnaryOperatorTestCase(-1, 2, -1, 2));
- testCases.add(new UnaryOperatorTestCase(1, -2, -1, 2));
+ testCases.add(new UnaryOperatorTestCase(1, -2, 1, -2));
testCases.add(new UnaryOperatorTestCase(-2, 4, -1, 2));
- testCases.add(new UnaryOperatorTestCase(2, -4, -1, 2));
+ testCases.add(new UnaryOperatorTestCase(2, -4, 1, -2));
- testCases.add(new UnaryOperatorTestCase(2, Integer.MIN_VALUE, -1, - (Integer.MIN_VALUE / 2)));
- testCases.add(new UnaryOperatorTestCase(Integer.MIN_VALUE, -2, - (Integer.MIN_VALUE / 2), 1));
+ testCases.add(new UnaryOperatorTestCase(2, Integer.MIN_VALUE, 1, Integer.MIN_VALUE / 2));
+ testCases.add(new UnaryOperatorTestCase(Integer.MIN_VALUE, -2, -Integer.MIN_VALUE / 2, -1));
return testCases;
}
@@ -158,8 +158,8 @@ class CommonTestCases {
List<UnaryOperatorTestCase> testCases = new ArrayList<>();
testCases.add(new UnaryOperatorTestCase(10, 21, 10, 21));
- testCases.add(new UnaryOperatorTestCase(-10, 21, 10, 21));
- testCases.add(new UnaryOperatorTestCase(10, -21, 10, 21));
+ testCases.add(new UnaryOperatorTestCase(-11, 23, 11, 23));
+ testCases.add(new UnaryOperatorTestCase(13, -24, -13, -24));
return testCases;
}
@@ -174,7 +174,7 @@ class CommonTestCases {
testCases.add(new UnaryOperatorTestCase(50, 75, 3, 2));
testCases.add(new UnaryOperatorTestCase(4, 3, 3, 4));
- testCases.add(new UnaryOperatorTestCase(-15, 47, -47, 15));
+ testCases.add(new UnaryOperatorTestCase(-15, 47, 47, -15));
testCases.add(new UnaryOperatorTestCase(Integer.MAX_VALUE, 1, 1, Integer.MAX_VALUE));
return testCases;
@@ -191,6 +191,11 @@ class CommonTestCases {
testCases.add(new UnaryOperatorTestCase(50, 75, -2, 3));
testCases.add(new UnaryOperatorTestCase(-50, 75, 2, 3));
testCases.add(new UnaryOperatorTestCase(Integer.MAX_VALUE - 1, Integer.MAX_VALUE, Integer.MIN_VALUE + 2, Integer.MAX_VALUE));
+ testCases.add(new UnaryOperatorTestCase(1, Integer.MIN_VALUE, -1, Integer.MIN_VALUE));
+
+ // XXX Failed by "BigFraction" (whose implementation differs from "Fraction").
+ // testCases.add(new UnaryOperatorTestCase(Integer.MIN_VALUE, Integer.MIN_VALUE, -1, 1));
+ // testCases.add(new UnaryOperatorTestCase(Integer.MIN_VALUE, 1, Integer.MIN_VALUE, -1));
return testCases;
}
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 2cf711c..ddf1011 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
@@ -41,13 +41,10 @@ public class FractionTest {
);
}
- // overflow
- Assertions.assertThrows(ArithmeticException.class,
- () -> Fraction.of(Integer.MIN_VALUE, -1)
- );
- Assertions.assertThrows(ArithmeticException.class,
- () -> Fraction.of(1, Integer.MIN_VALUE)
- );
+ // Special cases.
+ assertFraction(Integer.MIN_VALUE, -1, Fraction.of(Integer.MIN_VALUE, -1));
+ assertFraction(1, Integer.MIN_VALUE, Fraction.of(1, Integer.MIN_VALUE));
+ assertFraction(1, 1, Fraction.of(Integer.MIN_VALUE, Integer.MIN_VALUE));
}
@Test
@@ -229,13 +226,6 @@ public class FractionTest {
Fraction f = Fraction.of(testCase.operandNumerator, testCase.operandDenominator);
assertFraction(testCase.expectedNumerator, testCase.expectedDenominator, f.negate());
}
-
- {
- final Fraction f = Fraction.of(Integer.MIN_VALUE, 1);
- Assertions.assertThrows(ArithmeticException.class,
- f::negate
- );
- }
}
@Test
@@ -387,7 +377,7 @@ public class FractionTest {
{
Fraction b = Fraction.of(3, -7);
assertFraction(1, 1, b.pow(0));
- assertFraction(-3, 7, b.pow(1));
+ assertFraction(3, -7, b.pow(1));
assertFraction(-7, 3, b.pow(-1));
assertFraction(9, 49, b.pow(2));
assertFraction(49, 9, b.pow(-2));
@@ -395,7 +385,7 @@ public class FractionTest {
{
Fraction c = Fraction.of(0, -11);
- assertFraction(0, 1, c.pow(Integer.MAX_VALUE));
+ assertFraction(0, -1, c.pow(Integer.MAX_VALUE));
}
}
@@ -486,7 +476,7 @@ public class FractionTest {
Assertions.assertEquals("3", Fraction.of(6, 2).toString());
Assertions.assertEquals("2 / 3", Fraction.of(18, 27).toString());
Assertions.assertEquals("-10 / 11", Fraction.of(-10, 11).toString());
- Assertions.assertEquals("-10 / 11", Fraction.of(10, -11).toString());
+ Assertions.assertEquals("10 / -11", Fraction.of(10, -11).toString());
}
@Test