[
https://issues.apache.org/jira/browse/NUMBERS-131?focusedWorklogId=294826&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-294826
]
ASF GitHub Bot logged work on NUMBERS-131:
------------------------------------------
Author: ASF GitHub Bot
Created on: 14/Aug/19 16:18
Start Date: 14/Aug/19 16:18
Worklog Time Spent: 10m
Work Description: ericbarnhill commented on 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:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 294826)
Time Spent: 0.5h (was: 20m)
> Re-designing BigFraction.from(double, double, int, int)
> -------------------------------------------------------
>
> Key: NUMBERS-131
> URL: https://issues.apache.org/jira/browse/NUMBERS-131
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
> Affects Versions: 1.0
> Reporter: Heinrich Bohne
> Priority: Minor
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> The method {{BigFraction.from(double, double, int, int)}} can be improved in
> several ways:
> * It only allows a maximum denominator in the {{int}} range, which defies the
> purpose of having a {{BigFraction}} class in addition to the class
> {{Fraction}}. Since {{BigFraction}} is {{BigInteger}} based, it would only be
> natural to allow the maximum denominator to be passed as a {{BigInteger}}.
> * It only calculates the convergents of the simple continued fraction, but
> not its semi-convergents, so it doesn't necessarily produce the closest
> possible approximation within the given bounds.
> * The design is awkward. Making the method's behavior dependent on the values
> of its arguments is confusing, even the documentation acknowledges this.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
