diff --git a/src/java/org/apache/commons/lang/math/Fraction.java b/src/java/org/apache/commons/lang/math/Fraction.java index 91a82ef02..7628b9e12 100644 --- a/src/java/org/apache/commons/lang/math/Fraction.java +++ b/src/java/org/apache/commons/lang/math/Fraction.java @@ -66,7 +66,7 @@ import java.io.Serializable; * @author Stephen Colebourne * @author Tim O'Brien * @since 2.0 - * @version $Id: Fraction.java,v 1.8 2003/08/04 02:01:53 scolebourne Exp $ + * @version $Id: Fraction.java,v 1.9 2003/08/13 23:42:17 scolebourne Exp $ */ public final class Fraction extends Number implements Serializable, Comparable { @@ -161,6 +161,8 @@ public final class Fraction extends Number implements Serializable, Comparable { * @throws ArithmeticException if the denomiator is zero * @throws ArithmeticException if the denomiator is negative * @throws ArithmeticException if the numerator is negative + * @throws ArithmeticException if the resulting numerator exceeds + * Integer.MAX_VALUE */ public static Fraction getFraction(int whole, int numerator, int denominator) { if (denominator == 0) { @@ -172,12 +174,16 @@ public final class Fraction extends Number implements Serializable, Comparable { if (numerator < 0) { throw new ArithmeticException("The numerator must not be negative"); } + double numeratorValue = 0; if (whole < 0) { - numerator = whole * denominator - numerator; + numeratorValue = (double) whole * denominator - numerator; } else { - numerator = whole * denominator + numerator; + numeratorValue = (double) whole * denominator + numerator; } - return new Fraction(numerator, denominator); + if (Math.abs(numeratorValue) > Integer.MAX_VALUE) { + throw new ArithmeticException("Numerator too large to represent as an Integer."); + } + return new Fraction((int) numeratorValue, denominator); } /** @@ -199,30 +205,34 @@ public final class Fraction extends Number implements Serializable, Comparable { numerator = -numerator; denominator = -denominator; } - int gcd = greatestCommonDenominator(Math.abs(numerator), denominator); + int gcd = greatestCommonDivisor(Math.abs(numerator), denominator); return new Fraction(numerator / gcd, denominator / gcd); } /** *

Creates a Fraction instance from a double value.

* - *

This method uses the continued fraction algorithm.

+ *

This method uses the + * continued fraction algorithm, computing a maximum of + * 25 convergents and bounding the denominator by 10,000.

* * @param value the double value to convert * @return a new fraction instance that is close to the value - * @throws ArithmeticException if the value is infinite or NaN + * @throws ArithmeticException if |value| > Integer.MAX_VALUE + * or value = NaN * @throws ArithmeticException if the calculated denomiator is zero + * @throws ArithmeticException if the the algorithm does not converge */ public static Fraction getFraction(double value) { - if (Double.isInfinite(value) || Double.isNaN(value)) { - throw new ArithmeticException("The value must not be infinite or NaN"); - } int sign = (value < 0 ? -1 : 1); value = Math.abs(value); + if (value > Integer.MAX_VALUE || Double.isNaN(value)) { + throw new ArithmeticException + ("The value must not be greater than Integer.MAX_VALUE or NaN"); + } int wholeNumber = (int) value; value -= wholeNumber; - - // http://archives.math.utk.edu/articles/atuyl/confrac/ + int numer0 = 0; // the pre-previous int denom0 = 1; // the pre-previous int numer1 = 1; // the previous @@ -430,7 +440,7 @@ public final class Fraction extends Number implements Serializable, Comparable { * @return a new reduce fraction instance, or this if no simplification possible */ public Fraction reduce() { - int gcd = greatestCommonDenominator(Math.abs(numerator), denominator); + int gcd = greatestCommonDivisor(Math.abs(numerator), denominator); return Fraction.getFraction(numerator / gcd, denominator / gcd); } @@ -483,27 +493,39 @@ public final class Fraction extends Number implements Serializable, Comparable { * * @param power the power to raise the fraction to * @return this if the power is one, ONE if the power - * is zero or a new fraction instance raised to the appropriate power + * is zero (even if the fraction equals ZERO) or a new fraction instance + * raised to the appropriate power + * @throws ArithmeticException if the resulting numerator or denominator exceeds + * Integer.MAX_VALUE */ public Fraction pow(int power) { if (power == 1) { return this; } else if (power == 0) { return ONE; - } else if (power < 0) { - return getFraction((int) Math.pow(denominator, -power), (int) Math.pow(numerator, -power)); + } else { + double denominatorValue = Math.pow(denominator, power); + double numeratorValue = Math.pow(numerator, power); + if (numeratorValue > Integer.MAX_VALUE || denominatorValue > Integer.MAX_VALUE) { + throw new ArithmeticException("Integer overflow"); + } + if (power < 0) { + return getFraction((int) Math.pow(denominator, -power), + (int) Math.pow(numerator, -power)); + } + return getFraction((int) Math.pow(numerator, power), + (int) Math.pow(denominator, power)); } - return getFraction((int) Math.pow(numerator, power), (int) Math.pow(denominator, power)); } /** - *

Gets the greatest common denominator of two numbers.

+ *

Gets the greatest common divisor of two numbers.

* * @param number1 a positive number * @param number2 a positive number - * @return the greatest common denominator, never zero + * @return the greatest common divisor, never zero */ - private static int greatestCommonDenominator(int number1, int number2) { + private static int greatestCommonDivisor(int number1, int number2) { int remainder = number1 % number2; while (remainder != 0) { number1 = number2; @@ -517,15 +539,14 @@ public final class Fraction extends Number implements Serializable, Comparable { //------------------------------------------------------------------- /** - *

Adds the value of this fraction to another, returning the result.

- * - *

The implementation spots common cases of zero numerators and equal - * denominators. Otherwise, it uses (a/b) + (c/d) = (a*d + b*c) / (b*d) - * and then reduces the result.

+ *

Adds the value of this fraction to another, returning the result in + * reduced form.

* * @param fraction the fraction to add, must not be null * @return a Fraction instance with the resulting values * @throws IllegalArgumentException if the fraction is null + * @throws ArithmeticException if the resulting numerator or denominator exceeds + * Integer.MAX_VALUE */ public Fraction add(Fraction fraction) { if (fraction == null) { @@ -536,56 +557,46 @@ public final class Fraction extends Number implements Serializable, Comparable { } if (fraction.numerator == 0) { return this; + } + // Compute lcd explicitly to limit overflow + int gcd = greatestCommonDivisor(Math.abs(fraction.denominator), Math.abs(denominator)); + int thisResidue = denominator/gcd; + int thatResidue = fraction.denominator/gcd; + double denominatorValue = Math.abs((double) gcd * thisResidue * thatResidue); + double numeratorValue = (double) numerator * thatResidue + fraction.numerator * thisResidue; + if (Math.abs(numeratorValue) > Integer.MAX_VALUE || + Math.abs(denominatorValue) > Integer.MAX_VALUE) { + throw new ArithmeticException("Integer overflow"); } - if (denominator == fraction.denominator) { - return getReducedFraction(numerator + fraction.numerator, denominator); - } - return getReducedFraction( - numerator * fraction.denominator + denominator * fraction.numerator, - denominator * fraction.denominator - ); + return Fraction.getReducedFraction((int) numeratorValue, (int) denominatorValue); } /** *

Subtracts the value of another fraction from the value of this one, - * returning the result.

- * - *

The implementation spots common cases of zero numerators and equal - * denominators. Otherwise, it uses (a/b) - (c/d) = (a*d - b*c) / (b*d) - * and then reduces the result.

+ * returning the result in reduced form.

* * @param fraction the fraction to subtract, must not be null * @return a Fraction instance with the resulting values * @throws IllegalArgumentException if the fraction is null + * @throws ArithmeticException if the resulting numerator or denominator exceeds + * Integer.MAX_VALUE */ public Fraction subtract(Fraction fraction) { if (fraction == null) { throw new IllegalArgumentException("The fraction must not be null"); } - if (numerator == 0) { - return fraction.negate(); - } - if (fraction.numerator == 0) { - return this; - } - if (denominator == fraction.denominator) { - return getReducedFraction(numerator - fraction.numerator, denominator); - } - return getReducedFraction( - numerator * fraction.denominator - denominator * fraction.numerator, - denominator * fraction.denominator - ); + return add(fraction.negate()); } /** - *

Multiplies the value of this fraction by another, returning the result.

- * - *

The implementation uses (a/b)*(c/d) = (a*c)/(b*d) - * and then reduces the result.

+ *

Multiplies the value of this fraction by another, returning the result + * in reduced form.

* * @param fraction the fraction to multipy by, must not be null * @return a Fraction instance with the resulting values * @throws IllegalArgumentException if the fraction is null + * @throws ArithmeticException if the resulting numerator or denominator exceeds + * Integer.MAX_VALUE */ public Fraction multiplyBy(Fraction fraction) { if (fraction == null) { @@ -594,22 +605,25 @@ public final class Fraction extends Number implements Serializable, Comparable { if (numerator == 0 || fraction.numerator == 0) { return ZERO; } - return getReducedFraction( - numerator * fraction.numerator, - denominator * fraction.denominator - ); + double numeratorValue = (double) numerator * fraction.numerator; + double denominatorValue = (double) denominator * fraction.denominator; + if (Math.abs(numeratorValue) > Integer.MAX_VALUE || + Math.abs(denominatorValue) > Integer.MAX_VALUE) { + throw new ArithmeticException("Integer overflow"); + } + return getReducedFraction((int) numeratorValue, (int) denominatorValue); } /** - *

Divide the value of this fraction by another, returning the result.

- * - *

The implementation uses (a/b)/(c/d) = a/b * d/c = (a*d)/(b*c) - * and then reduces the result.

+ *

Divide the value of this fraction by another, returning the result + * in reduced form.

* * @param fraction the fraction to divide by, must not be null * @return a Fraction instance with the resulting values * @throws IllegalArgumentException if the fraction is null * @throws ArithmeticException if the fraction to divide by is zero + * @throws ArithmeticException if the resulting numerator or denominator exceeds + * Integer.MAX_VALUE */ public Fraction divideBy(Fraction fraction) { if (fraction == null) { @@ -620,11 +634,14 @@ public final class Fraction extends Number implements Serializable, Comparable { } if (numerator == 0) { return ZERO; + } + double numeratorValue = (double) numerator * fraction.denominator; + double denominatorValue = (double) denominator * fraction.numerator; + if (Math.abs(numeratorValue) > Integer.MAX_VALUE || + Math.abs(denominatorValue) > Integer.MAX_VALUE) { + throw new ArithmeticException("Integer overflow"); } - return getReducedFraction( - numerator * fraction.denominator, - denominator * fraction.numerator - ); + return getReducedFraction((int) numeratorValue, (int) denominatorValue); } // Basics @@ -668,7 +685,7 @@ public final class Fraction extends Number implements Serializable, Comparable { *

Compares this object to another based on size.

* * @param object the object to compare to - * @return -ve if this is less, 0 if equal, +ve if greater + * @return -1 if this is less, 0 if equal, +1 if greater * @throws ClassCastException if the object is not a Fraction * @throws NullPointerException if the object is null */ diff --git a/src/test/org/apache/commons/lang/math/FractionTest.java b/src/test/org/apache/commons/lang/math/FractionTest.java index 15bbfba22..fa54cce30 100644 --- a/src/test/org/apache/commons/lang/math/FractionTest.java +++ b/src/test/org/apache/commons/lang/math/FractionTest.java @@ -60,11 +60,11 @@ import junit.framework.TestSuite; * Test cases for the {@link Fraction} classes. * * @author Stephen Colebourne - * @version $Id: FractionTest.java,v 1.3 2003/08/04 02:01:53 scolebourne Exp $ + * @version $Id: FractionTest.java,v 1.4 2003/08/13 23:42:17 scolebourne Exp $ */ public class FractionTest extends TestCase { - private static final int SKIP = 17; + private static final int SKIP = 53; public FractionTest(String name) { super(name); @@ -165,14 +165,17 @@ public class FractionTest extends TestCase { // zero denominator try { f = Fraction.getFraction(1, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(2, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(-3, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} } @@ -200,14 +203,17 @@ public class FractionTest extends TestCase { // negatives try { f = Fraction.getFraction(1, -6, -10); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(1, -6, -10); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(1, -6, -10); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} // negative whole @@ -217,27 +223,43 @@ public class FractionTest extends TestCase { try { f = Fraction.getFraction(-1, -6, 10); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(-1, 6, -10); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(-1, -6, -10); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} // zero denominator try { f = Fraction.getFraction(0, 1, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(1, 2, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(-1, -3, 0); + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} + + try { + f = Fraction.getFraction(Integer.MAX_VALUE, 1, 2); + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} + + try { + f = Fraction.getFraction(-Integer.MAX_VALUE, 1, 2); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} } @@ -279,14 +301,17 @@ public class FractionTest extends TestCase { // zero denominator try { f = Fraction.getReducedFraction(1, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getReducedFraction(2, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getReducedFraction(-3, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} // reduced @@ -316,14 +341,22 @@ public class FractionTest extends TestCase { try { f = Fraction.getFraction(Double.NaN); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(Double.POSITIVE_INFINITY); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} try { f = Fraction.getFraction(Double.NEGATIVE_INFINITY); + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} + + try { + f = Fraction.getFraction((double) Integer.MAX_VALUE + 1); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} // zero @@ -377,7 +410,7 @@ public class FractionTest extends TestCase { assertEquals(f2.getDenominator(), f.getDenominator()); } } - // save time by skipping some tests! + // save time by skipping some tests! ( for (int i = 1001; i <= 10000; i+=SKIP) { // denominator for (int j = 1; j <= i; j++) { // numerator try { @@ -396,9 +429,11 @@ public class FractionTest extends TestCase { public void testFactory_String() { try { Fraction.getFraction(null); + fail("expecting ArithmeticException"); } catch (IllegalArgumentException ex) {} } + public void testFactory_String_double() { Fraction f = null; @@ -422,6 +457,11 @@ public class FractionTest extends TestCase { f = Fraction.getFraction("2.3R"); } catch (NumberFormatException ex) {} + try { + f = Fraction.getFraction("2147483648"); // too big + fail("Expecting NumberFormatException"); + } catch (NumberFormatException ex) {} + try { f = Fraction.getFraction("."); } catch (NumberFormatException ex) {} @@ -448,26 +488,32 @@ public class FractionTest extends TestCase { try { f = Fraction.getFraction("2 3"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction("a 3"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction("2 b/4"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction("2 "); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction(" 3"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction(" "); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} } @@ -500,18 +546,22 @@ public class FractionTest extends TestCase { try { f = Fraction.getFraction("2/d"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction("2e/3"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction("2/"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} try { f = Fraction.getFraction("/"); + fail("expecting NumberFomatException"); } catch (NumberFormatException ex) {} } @@ -625,6 +675,12 @@ public class FractionTest extends TestCase { f = f.pow(-2); assertEquals(25, f.getNumerator()); assertEquals(9, f.getDenominator()); + + f = Fraction.getFraction(Integer.MAX_VALUE); + try { + f = f.pow(2); + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} } public void testAdd() { @@ -656,12 +712,24 @@ public class FractionTest extends TestCase { assertEquals(-1, f.getNumerator()); assertEquals(5, f.getDenominator()); + f1 = Fraction.getFraction(Integer.MAX_VALUE - 1, 1); + f2 = Fraction.ONE; + f = f1.add(f2); + assertEquals(Integer.MAX_VALUE, f.getNumerator()); + assertEquals(1, f.getDenominator()); + f1 = Fraction.getFraction(3, 5); f2 = Fraction.getFraction(1, 2); f = f1.add(f2); assertEquals(11, f.getNumerator()); assertEquals(10, f.getDenominator()); + f1 = Fraction.getFraction(3, 8); + f2 = Fraction.getFraction(1, 6); + f = f1.add(f2); + assertEquals(13, f.getNumerator()); + assertEquals(24, f.getDenominator()); + f1 = Fraction.getFraction(0, 5); f2 = Fraction.getFraction(1, 5); f = f1.add(f2); @@ -671,7 +739,26 @@ public class FractionTest extends TestCase { try { f.add(null); + fail("expecting IllegalArgumentException"); } catch (IllegalArgumentException ex) {} + + f1 = Fraction.getFraction(Integer.MAX_VALUE - 1, 1); + f2 = Fraction.ONE; + f = f1.add(f2); + assertEquals(Integer.MAX_VALUE, f.getNumerator()); + assertEquals(1, f.getDenominator()); + + try { + f = f.add(Fraction.ONE); // should overflow + fail("expecting ArithmeticException but got: " + f.toString()); + } catch (ArithmeticException ex) {} + + try { + f= Fraction.getFraction(-Integer.MAX_VALUE, 1); + f = f.add(f); + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} + } public void testSubtract() { @@ -728,7 +815,16 @@ public class FractionTest extends TestCase { try { f.subtract(null); + fail("expecting IllegalArgumentException"); } catch (IllegalArgumentException ex) {} + + try { + f1 = Fraction.getFraction(1, Integer.MAX_VALUE); + f2 = Fraction.getFraction(1, Integer.MAX_VALUE - 1); + f = f1.subtract(f2); + fail("expecting ArithmeticException"); //should overflow + } catch (ArithmeticException ex) {} + } public void testMultiply() { @@ -759,9 +855,28 @@ public class FractionTest extends TestCase { f = f1.multiplyBy(f2); assertSame(Fraction.ZERO, f); + f1 = Fraction.getFraction(2, 7); + f2 = Fraction.ONE; + f = f1.multiplyBy(f2); + assertEquals(2, f.getNumerator()); + assertEquals(7, f.getDenominator()); + try { f.multiplyBy(null); + fail("expecting IllegalArgumentException"); } catch (IllegalArgumentException ex) {} + + try { + f1 = Fraction.getFraction(1, Integer.MAX_VALUE); + f = f1.multiplyBy(f1); // should overflow + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} + + try { + f1 = Fraction.getFraction(1, -Integer.MAX_VALUE); + f = f1.multiplyBy(f1); // should overflow + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} } public void testDivide() { @@ -779,6 +894,7 @@ public class FractionTest extends TestCase { f2 = Fraction.ZERO; try { f = f1.divideBy(f2); + fail("expecting ArithmeticException"); } catch (ArithmeticException ex) {} f1 = Fraction.getFraction(0, 5); @@ -786,9 +902,32 @@ public class FractionTest extends TestCase { f = f1.divideBy(f2); assertSame(Fraction.ZERO, f); + f1 = Fraction.getFraction(2, 7); + f2 = Fraction.ONE; + f = f1.divideBy(f2); + assertEquals(2, f.getNumerator()); + assertEquals(7, f.getDenominator()); + + f1 = Fraction.getFraction(1, Integer.MAX_VALUE); + f = f1.divideBy(f1); + assertEquals(1, f.getNumerator()); + assertEquals(1, f.getDenominator()); + try { f.divideBy(null); + fail("IllegalArgumentException"); } catch (IllegalArgumentException ex) {} + + try { + f1 = Fraction.getFraction(1, Integer.MAX_VALUE); + f = f1.divideBy(f1.invert()); // should overflow + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} + try { + f1 = Fraction.getFraction(1, -Integer.MAX_VALUE); + f = f1.divideBy(f1.invert()); // should overflow + fail("expecting ArithmeticException"); + } catch (ArithmeticException ex) {} } public void testEquals() { @@ -834,10 +973,12 @@ public class FractionTest extends TestCase { try { f1.compareTo(null); + fail("expecting NullPointerException"); } catch (NullPointerException ex) {} try { f1.compareTo(new Object()); + fail("expecting ClassCastException"); } catch (ClassCastException ex) {} f2 = Fraction.getFraction(2, 5);