This is in fact a change of behaviour, since passing zeros to this method previously caused it to loop forever. As its private though I looked at the calling methods to see if thats possible - getReducedFraction(), add(), subtract() and multiplyBy() are all protected against this (i.e. don't call greatestCommonDivisor() with a zero numerator and zero denominator is not possible for a Fraction [throws ArithmeticException]) - but reduce() is not protected, so prior to this change there was a bug which is now fixed.
We should at least document the bug fix with a Jira issue - but I think that although this fixed a bug in this case, it could have been the other way round and we sould ensure that changes to existing methods are covered by test cases however minor. Niall On Nov 27, 2007 5:25 PM, <[EMAIL PROTECTED]> wrote: > Author: mbenson > Date: Tue Nov 27 09:24:59 2007 > New Revision: 598705 > > URL: http://svn.apache.org/viewvc?rev=598705&view=rev > Log: > avoid unnecessary work; remove commented code > > Modified: > > commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java > > Modified: > commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java > URL: > http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java?rev=598705&r1=598704&r2=598705&view=diff > ============================================================================== > --- > commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java > (original) > +++ > commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java > Tue Nov 27 09:24:59 2007 > @@ -567,11 +567,14 @@ > * @return the greatest common divisor, never zero > */ > private static int greatestCommonDivisor(int u, int v) { > + //if either op. is abs 0 or 1, return 1: > + if (Math.abs(u) <= 1 || Math.abs(v) <= 1) { > + return 1; > + } > // keep u and v negative, as negative integers range down to > // -2^31, while positive numbers can only be as large as 2^31-1 > // (i.e. we can't necessarily negate a negative number without > // overflow) > - /* assert u!=0 && v!=0; */ > if (u>0) { u=-u; } // make u negative > if (v>0) { v=-v; } // make v negative > // B1. [Find power of 2] > > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
