Fraction/BigFraction to validate input double in factory constructors
The double must not be nan or infinite.
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 9a969c9..152a33e 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
@@ -47,6 +47,9 @@
/** Serializable version identifier. */
private static final long serialVersionUID = 20190701L;
+ /** Message for non-finite input double argument to factory constructors. */
+ private static final String NOT_FINITE = "Not finite: ";
+
/** The numerator of this fraction reduced to lowest terms. */
private final BigInteger numerator;
@@ -110,12 +113,17 @@
* @param maxDenominator Maximum denominator value allowed.
* @param maxIterations Maximum number of convergents.
* @return a new instance.
+ * @throws IllegalArgumentException if the given {@code value} is NaN or infinite.
* @throws ArithmeticException if the continued fraction failed to converge.
*/
private static BigFraction from(final double value,
final double epsilon,
final int maxDenominator,
final int maxIterations) {
+ if (!Double.isFinite(value)) {
+ throw new IllegalArgumentException(NOT_FINITE + value);
+ }
+
final long overflow = Integer.MAX_VALUE;
double r0 = value;
long a0 = (long) Math.floor(r0);
@@ -187,7 +195,7 @@
/**
* Create a fraction given the double value.
* <p>
- * This factory method behaves <em>differently</em> from
+ * This factory method behaves <em>differently</em> to the method
* {@link #from(double, double, int)}. It converts the double value
* exactly, considering its internal bits representation. This works for all
* values except NaN and infinities and does not requires any loop or
@@ -209,11 +217,8 @@
* @see #from(double,double,int)
*/
public static BigFraction from(final double value) {
- if (Double.isNaN(value)) {
- throw new IllegalArgumentException("Cannot convert NaN value");
- }
- if (Double.isInfinite(value)) {
- throw new IllegalArgumentException("Cannot convert infinite value");
+ if (!Double.isFinite(value)) {
+ throw new IllegalArgumentException(NOT_FINITE + value);
}
final long bits = Double.doubleToLongBits(value);
@@ -266,6 +271,7 @@
* @param epsilon Maximum error allowed. The resulting fraction is within
* {@code epsilon} of {@code value}, in absolute terms.
* @param maxIterations Maximum number of convergents.
+ * @throws IllegalArgumentException if the given {@code value} is NaN or infinite.
* @throws ArithmeticException if the continued fraction failed to converge.
* @return a new instance.
*
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 8135438..0ec2009 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
@@ -47,6 +47,9 @@
/** The default epsilon used for convergence. */
private static final double DEFAULT_EPSILON = 1e-5;
+ /** Message for non-finite input double argument to factory constructors. */
+ private static final String NOT_FINITE = "Not finite: ";
+
/** The numerator of this fraction reduced to lowest terms. */
private final int numerator;
@@ -55,7 +58,7 @@
/**
- * Constructs an instance.
+ * Private constructor: Instances are created using factory methods.
*
* @param num Numerator.
* @param den Denominator.
@@ -98,16 +101,17 @@
* <p>
* NOTE: This constructor is called with:
* <ul>
- * <li>EITHER a valid epsilon value and the maxDenominator set to Integer.MAX_VALUE
- * (that way the maxDenominator has no effect).
- * <li>OR a valid maxDenominator value and the epsilon value set to zero
- * (that way epsilon only has effect if there is an exact match before
- * the maxDenominator value is reached).
+ * <li>EITHER a valid epsilon value and the maxDenominator set to
+ * Integer.MAX_VALUE (that way the maxDenominator has no effect)
+ * <li>OR a valid maxDenominator value and the epsilon value set to
+ * zero (that way epsilon only has effect if there is an exact
+ * match before the maxDenominator value is reached).
* </ul>
* <p>
- * It has been done this way so that the same code can be (re)used for both
- * scenarios. However this could be confusing to users if it were part of
- * the public API and this constructor should therefore remain PRIVATE.
+ * It has been done this way so that the same code can be reused for
+ * both scenarios. However this could be confusing to users if it
+ * were part of the public API and this method should therefore remain
+ * PRIVATE.
* </p>
*
* <p>
@@ -115,18 +119,23 @@
* https://issues.apache.org/jira/browse/MATH-181
* </p>
*
- * @param value the double value to convert to a fraction.
- * @param epsilon maximum error allowed. The resulting fraction is
- * within {@code epsilon} of {@code value}, in absolute terms.
- * @param maxDenominator maximum denominator value allowed.
- * @param maxIterations maximum number of convergents
- * @throws ArithmeticException if the continued fraction failed
- * to converge.
+ * @param value Value to convert to a fraction.
+ * @param epsilon Maximum error allowed.
+ * The resulting fraction is within {@code epsilon} of {@code value},
+ * in absolute terms.
+ * @param maxDenominator Maximum denominator value allowed.
+ * @param maxIterations Maximum number of convergents.
+ * @throws IllegalArgumentException if the given {@code value} is NaN or infinite.
+ * @throws ArithmeticException if the continued fraction failed to converge.
*/
private Fraction(final double value,
final double epsilon,
final int maxDenominator,
final int maxIterations) {
+ if (!Double.isFinite(value)) {
+ throw new IllegalArgumentException(NOT_FINITE + value);
+ }
+
final long overflow = Integer.MAX_VALUE;
double r0 = value;
long a0 = (long)Math.floor(r0);
@@ -201,6 +210,7 @@
* Create a fraction given the double value.
*
* @param value Value to convert to a fraction.
+ * @throws IllegalArgumentException if the given {@code value} is NaN or infinite.
* @throws ArithmeticException if the continued fraction failed to
* converge.
* @return a new instance.
@@ -223,6 +233,7 @@
* @param epsilon maximum error allowed. The resulting fraction is within
* {@code epsilon} of {@code value}, in absolute terms.
* @param maxIterations maximum number of convergents
+ * @throws IllegalArgumentException if the given {@code value} is NaN or infinite.
* @throws ArithmeticException if the continued fraction failed to
* converge.
* @return a new instance.
@@ -245,6 +256,7 @@
*
* @param value the double value to convert to a fraction.
* @param maxDenominator The maximum allowed value for denominator
+ * @throws IllegalArgumentException if the given {@code value} is NaN or infinite.
* @throws ArithmeticException if the continued fraction failed to
* converge.
* @return a new instance.
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 62057a2..d2da883 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
@@ -128,6 +128,18 @@
assertFraction(1, 2, BigFraction.from(0.5000000001, 10));
}
+ @Test
+ public void testDoubleConstructorThrowsWithNonFinite() {
+ final double eps = 1e-5;
+ final int maxIterations = Integer.MAX_VALUE;
+ final int maxDenominator = Integer.MAX_VALUE;
+ for (final double value : new double[] {Double.NaN, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}) {
+ Assertions.assertThrows(IllegalArgumentException.class, () -> BigFraction.from(value));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> BigFraction.from(value, eps, maxIterations));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> BigFraction.from(value, maxDenominator));
+ }
+ }
+
// MATH-1029
@Test
public void testPositiveValueOverflow() {
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 9d19663..5534d2e 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
@@ -67,6 +67,18 @@
}
}
+ @Test
+ public void testDoubleConstructorThrowsWithNonFinite() {
+ final double eps = 1e-5;
+ final int maxIterations = Integer.MAX_VALUE;
+ final int maxDenominator = Integer.MAX_VALUE;
+ for (final double value : new double[] {Double.NaN, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}) {
+ Assertions.assertThrows(IllegalArgumentException.class, () -> Fraction.from(value));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> Fraction.from(value, eps, maxIterations));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> Fraction.from(value, maxDenominator));
+ }
+ }
+
// MATH-181
@Test
public void testDigitLimitConstructor() throws Exception {
