Hello! I think a re-design of the factory method BigFraction.from(double, double, int, int) is in order, because I see several problems with it:
First, having a separate fraction class intended to overcome the limitations of the int range with a factory method (formerly a constructor) for approximating double values that can only produce denominators within the int range because it has been copy-pasted from Fraction (where this code is still a constructor) seems a bit like a joke. I think it would be more useful to have this method accept a BigInteger as an upper bound for the denominator instead of an int. Second, the method only calculates the convergents of the corresponding continued fraction, but never its semi-convergents, so it doesn't necessarily produce the best rational approximation of the double number within the given bounds. For example, the test method BigFractionTest.testDigitLimitConstructor() asserts that the method calculates 3/5 as an approximation of 0.6152 with the upper bound for the denominator set to 9, but 5/8 = 0.625 is closer to 0.6152 than 3/5 = 0.6. Since the method is already using continued fractions to approximate fractional numbers, I think it would be a pity if it didn't take advantage of them for all that they're worth. Finally, the documentation of the method rightfully acknowledges the latter's confusing design, with the method's general behavior being dependent on some of its arguments and the validity of these arguments also being dependent on each other. However, a better way to solve this problem than to simply hide the design from the public would be to improve it, e.g. by extracting the functionality that is common to both the "maxDenominator mode" and the epsilon mode (which is the calculation of the continued fraction), and separating the differences in the functionality of the two modes into distinct methods that call the common functionality. My suggestion for the third point above would be to create a separate class (not necessarily public) that provides an interface for calculating simple continued fractions and their convergents (I see that there's an abstract class ContinuedFraction, but I don't think it will be useful, because all the methods only return double values, and the class also requires that all coefficients can be explicitly calculated based on their index). The class would ideally be able to calculate the continued fraction dynamically/lazily, because only a limited number of coefficients are needed to approximate a fractional number within given bounds. What I think could be useful is if the class stores a list of the coefficients internally in addition to the current and previous convergent (two consecutive convergents are needed to calculate the next one recursively based on the next coefficient), and has methods like addCoefficient(BigInteger) and removeLastCoefficient() for building a continued fraction, and also a static method like coefficientsOf(BigFraction) that returns an Iterator<BigInteger> that computes the coefficients only as they are queried through the iterator, so that they can then be passed to addCoefficient(BigInteger). The maxDenominator factory method could then just iterate over the coefficients of the continued fraction representation of the passed double and build the continued fraction from them until the denominator of the current convergent exceeds the upper bound, and the epsilon method could iterate over the coefficients of both the lower and upper bound's continued fraction representation until the coefficients start to differ, at which point it can build the continued fraction of the close enough approximation from all coefficients at once (this would also prevent any loss of precision when repeatedly performing arithmetic operations with floating-point values). Furthermore, this code could not only be used by the approximation factory methods in BigFraction, but also by those in Fraction, possibly adjusted so that not only the denominator must be within a given bound, but also the numerator needs to be within the int range. Any opinions or objections? --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
