[GitHub] [commons-numbers] ericbarnhill commented on a change in pull request #66: [NUMBERS-131] Re-write implementations of double approximation factory methods in BigFraction

[GitBox <gi...@apache.org>]

ericbarnhill commented on a change in pull request #66: [NUMBERS-131] Re-write implementations of double approximation factory methods in BigFraction
URL: https://github.com/apache/commons-numbers/pull/66#discussion_r313963822
 
 

 ##########
 File path: commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
 ##########
 @@ -320,14 +320,103 @@ public static BigFraction from(final double value,
      *
      * @param value Value to convert to a fraction.
      * @param maxDenominator Maximum allowed value for denominator.
-     * @throws ArithmeticException if the continued fraction failed to converge.
+     * @throws IllegalArgumentException if the given {@code value} is NaN or
+     *         infinite, or if {@code maxDenominator < 1}
+     * @throws NullPointerException if {@code maxDenominator} is {@code null}
      * @return a new instance.
      *
      * @see #from(double,double,int)
      */
     public static BigFraction from(final double value,
-                                   final int maxDenominator) {
-        return from(value, 0, maxDenominator, 100);
+                                   final BigInteger maxDenominator) {
+        if (maxDenominator.signum() != 1) {
+            throw new IllegalArgumentException("Upper bound for denominator must be positive: " + maxDenominator);
+        }
+
+        /*
+         * Required facts:
 
 Review comment:
   At some point it will probably be better to move discussions like this to a wiki or other documentation, and just link to it here.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services