diff --git a/pom.xml b/pom.xml index 320c3fdce..1675d0883 100644 --- a/pom.xml +++ b/pom.xml @@ -353,6 +353,9 @@ Xiaogang Zhang + + Chris Popp + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b6baf47b7..bbb67e7a6 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -54,6 +54,9 @@ If the output is not quite correct, check for invisible trailing spaces! + + Removed unnecessary allocations in "BigFraction" (package "o.a.c.m.fraction"). + Fixed error in computing discrete distribution of D statistics for small-sample 2-sample Kolmogorov-Smirnov tests. Error was causing incorrect p-values returned diff --git a/src/main/java/org/apache/commons/math4/fraction/BigFraction.java b/src/main/java/org/apache/commons/math4/fraction/BigFraction.java index 078682d5a..1be1eeac3 100644 --- a/src/main/java/org/apache/commons/math4/fraction/BigFraction.java +++ b/src/main/java/org/apache/commons/math4/fraction/BigFraction.java @@ -119,10 +119,10 @@ public class BigFraction public BigFraction(BigInteger num, BigInteger den) { MathUtils.checkNotNull(num, LocalizedFormats.NUMERATOR); MathUtils.checkNotNull(den, LocalizedFormats.DENOMINATOR); - if (BigInteger.ZERO.equals(den)) { + if (den.signum() == 0) { throw new ZeroException(LocalizedFormats.ZERO_DENOMINATOR); } - if (BigInteger.ZERO.equals(num)) { + if (num.signum() == 0) { numerator = BigInteger.ZERO; denominator = BigInteger.ONE; } else { @@ -135,7 +135,7 @@ public class BigFraction } // move sign to numerator - if (BigInteger.ZERO.compareTo(den) > 0) { + if (den.signum() == -1) { num = num.negate(); den = den.negate(); } @@ -450,7 +450,7 @@ public class BigFraction * @return the absolute value as a {@link BigFraction}. */ public BigFraction abs() { - return (BigInteger.ZERO.compareTo(numerator) <= 0) ? this : negate(); + return (numerator.signum() == 1) ? this : negate(); } /** @@ -467,6 +467,14 @@ public class BigFraction */ public BigFraction add(final BigInteger bg) throws NullArgumentException { MathUtils.checkNotNull(bg); + + if (numerator.signum() == 0) { + return new BigFraction(bg); + } + if (bg.signum() == 0) { + return this; + } + return new BigFraction(numerator.add(denominator.multiply(bg)), denominator); } @@ -514,9 +522,12 @@ public class BigFraction if (fraction == null) { throw new NullArgumentException(LocalizedFormats.FRACTION); } - if (ZERO.equals(fraction)) { + if (fraction.numerator.signum() == 0) { return this; } + if (numerator.signum() == 0) { + return fraction; + } BigInteger num = null; BigInteger den = null; @@ -528,6 +539,11 @@ public class BigFraction num = (numerator.multiply(fraction.denominator)).add((fraction.numerator).multiply(denominator)); den = denominator.multiply(fraction.denominator); } + + if (num.signum() == 0) { + return ZERO; + } + return new BigFraction(num, den); } @@ -599,6 +615,16 @@ public class BigFraction */ @Override public int compareTo(final BigFraction object) { + int lhsSigNum = numerator.signum(); + int rhsSigNum = object.numerator.signum(); + + if (lhsSigNum != rhsSigNum) { + return (lhsSigNum > rhsSigNum) ? 1 : -1; + } + if (lhsSigNum == 0) { + return 0; + } + BigInteger nOd = numerator.multiply(object.denominator); BigInteger dOn = denominator.multiply(object.numerator); return nOd.compareTo(dOn); @@ -619,9 +645,12 @@ public class BigFraction if (bg == null) { throw new NullArgumentException(LocalizedFormats.FRACTION); } - if (BigInteger.ZERO.equals(bg)) { + if (bg.signum() == 0) { throw new MathArithmeticException(LocalizedFormats.ZERO_DENOMINATOR); } + if (numerator.signum() == 0) { + return ZERO; + } return new BigFraction(numerator, denominator.multiply(bg)); } @@ -669,9 +698,12 @@ public class BigFraction if (fraction == null) { throw new NullArgumentException(LocalizedFormats.FRACTION); } - if (BigInteger.ZERO.equals(fraction.numerator)) { + if (fraction.numerator.signum() == 0) { throw new MathArithmeticException(LocalizedFormats.ZERO_DENOMINATOR); } + if (numerator.signum() == 0) { + return ZERO; + } return multiply(fraction.reciprocal()); } @@ -873,6 +905,9 @@ public class BigFraction if (bg == null) { throw new NullArgumentException(); } + if (numerator.signum() == 0 || bg.signum() == 0) { + return ZERO; + } return new BigFraction(bg.multiply(numerator), denominator); } @@ -888,6 +923,10 @@ public class BigFraction */ @Override public BigFraction multiply(final int i) { + if (i == 0 || numerator.signum() == 0) { + return ZERO; + } + return multiply(BigInteger.valueOf(i)); } @@ -902,6 +941,10 @@ public class BigFraction * @return a {@link BigFraction} instance with the resulting values. */ public BigFraction multiply(final long l) { + if (l == 0 || numerator.signum() == 0) { + return ZERO; + } + return multiply(BigInteger.valueOf(l)); } @@ -920,8 +963,8 @@ public class BigFraction if (fraction == null) { throw new NullArgumentException(LocalizedFormats.FRACTION); } - if (numerator.equals(BigInteger.ZERO) || - fraction.numerator.equals(BigInteger.ZERO)) { + if (numerator.signum() == 0 || + fraction.numerator.signum() == 0) { return ZERO; } return new BigFraction(numerator.multiply(fraction.numerator), @@ -965,6 +1008,13 @@ public class BigFraction * @return thisexponent. */ public BigFraction pow(final int exponent) { + if (exponent == 0) { + return ONE; + } + if (numerator.signum() == 0) { + return this; + } + if (exponent < 0) { return new BigFraction(denominator.pow(-exponent), numerator.pow(-exponent)); } @@ -982,6 +1032,13 @@ public class BigFraction * @return thisexponent as a BigFraction. */ public BigFraction pow(final long exponent) { + if (exponent == 0) { + return ONE; + } + if (numerator.signum() == 0) { + return this; + } + if (exponent < 0) { return new BigFraction(ArithmeticUtils.pow(denominator, -exponent), ArithmeticUtils.pow(numerator, -exponent)); @@ -1001,7 +1058,14 @@ public class BigFraction * @return thisexponent as a BigFraction. */ public BigFraction pow(final BigInteger exponent) { - if (exponent.compareTo(BigInteger.ZERO) < 0) { + if (exponent.signum() == 0) { + return ONE; + } + if (numerator.signum() == 0) { + return this; + } + + if (exponent.signum() == -1) { final BigInteger eNeg = exponent.negate(); return new BigFraction(ArithmeticUtils.pow(denominator, eNeg), ArithmeticUtils.pow(numerator, eNeg)); @@ -1047,7 +1111,12 @@ public class BigFraction */ public BigFraction reduce() { final BigInteger gcd = numerator.gcd(denominator); - return new BigFraction(numerator.divide(gcd), denominator.divide(gcd)); + + if (BigInteger.ONE.compareTo(gcd) < 0) { + return new BigFraction(numerator.divide(gcd), denominator.divide(gcd)); + } else { + return this; + } } /** @@ -1064,6 +1133,13 @@ public class BigFraction if (bg == null) { throw new NullArgumentException(); } + if (bg.signum() == 0) { + return this; + } + if (numerator.signum() == 0) { + return new BigFraction(bg.negate()); + } + return new BigFraction(numerator.subtract(denominator.multiply(bg)), denominator); } @@ -1108,9 +1184,12 @@ public class BigFraction if (fraction == null) { throw new NullArgumentException(LocalizedFormats.FRACTION); } - if (ZERO.equals(fraction)) { + if (fraction.numerator.signum() == 0) { return this; } + if (numerator.signum() == 0) { + return fraction.negate(); + } BigInteger num = null; BigInteger den = null;