svn commit: r598705 - /commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java

[mb...@apache.org]

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]

Re: svn commit: r598705 - /commons/proper/lang/trunk/src/java/org/apache/commons/lang/math/Fraction.java

[Niall Pemberton <ni...@gmail.com>]

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,  <mb...@apache.org> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org