On Sat, 29 May 2004, Stephen Colebourne wrote:
However, we do have to be careful about introducing incompatabilities into the class. This Fraction class should follow the 80/20 rule, and maintain simplicity when possible. If necessary, a better implementation in the commons-math project could then be created. If the incompatability only affects fractions with large numerators/denominators then that might be OK introducing an incompatability.
The existing add(), subtract(), and multiplyBy() and divide() code (as far as I can tell) all returned reduced fractions, so existing arithmetic code should largely be unaffected. The primary visible difference is that before Fraction.getFraction(2,4).equals(Fraction.getFraction(1,2))==false and now it would be true. I view the previous behavior as "surprising" and a bug, honestly.
I am inclined to agree here; though I am not sure exactly what the original author's intention was and how people may be using the class now. The fact that the arithmetic operations return reduced fractions makes it hard to understand why equals works the way that it does now.
The previous implementation was very dangerous in so far as it could silently corrupt your fraction due to overflow in its temporary values, even if the result "should have" fit into an int. Now these corner cases have been fixed. This is what projects like commons promise: robust implementations of common code where all of the nasty gotchas have been thought-through and fixed. I hope that my patch accomplishes this for Fraction.
Hopefully the math project crew can review the current implementation and patch as well.
I will review in detail (posting comments to the bug report); but based on a quick review, the changes look good to me.
One thing that we might want to consider, given the magnitude of the changes, is deprecating this in [lang] and moving it to [math]. That way, among other things, the (better) continued fraction implementation in [math] could be used in getFraction(double). The improved gcd and checked integer arithmetic methods in the patch might also make good additions to o.a.c.math.MathUtils.
One more thing that we need to verify is that the Knuth algorithms are not patented or protected a la _Numerical Recipes_. Given Knuth's public position on software patents, I would be surprised if this were the case; but we should verify that neither Knuth's publishers nor _NR_ have claims that could encumber the implementations in the patch as derivative work prior to committing.
Phil
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
