commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aherb...@apache.org
Subject [commons-numbers] 01/08: Clean-up Fraction hashcode.
Date Wed, 08 Apr 2020 21:17:19 GMT
This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit c7a2734c4db92354d34a6feec64d89a3055cc600
Author: Alex Herbert <aherbert@apache.org>
AuthorDate: Wed Apr 8 21:09:23 2020 +0100

    Clean-up Fraction hashcode.
    
    Removed reference to Arrays.hashCode in the comments.
    
    Changed computation to be the hash generated from Array.hashCode with
    the magnitude of the numerator and the denominator then multiplied by
    the sign.
---
 .../java/org/apache/commons/numbers/fraction/BigFraction.java    | 7 +++----
 .../main/java/org/apache/commons/numbers/fraction/Fraction.java  | 9 ++++-----
 .../org/apache/commons/numbers/fraction/BigFractionTest.java     | 6 +++---
 .../java/org/apache/commons/numbers/fraction/FractionTest.java   | 6 +++---
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
index 3962e2e..d5c52b1 100644
--- a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
+++ b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
@@ -1020,16 +1020,15 @@ public final class BigFraction
     @Override
     public int hashCode() {
         // Incorporate the sign and absolute values of the numerator and denominator.
-        // Equivalent to
-        // Arrays.hashCode(new int[] {signum(), numerator.abs(), denominator.abs()})
+        // Equivalent to:
         // int hash = 1;
-        // hash = 31 * hash + signum();
         // hash = 31 * hash + numerator.abs().hashCode();
         // hash = 31 * hash + denominator.abs().hashCode();
+        // hash = hash * signum()
         // Note: BigInteger.hashCode() * BigInteger.signum() == BigInteger.abs().hashCode().
         final int numS = numerator.signum();
         final int denS = denominator.signum();
-        return 31 * (31 * (31 + numS * denS) + numerator.hashCode() * numS) + denominator.hashCode()
* denS;
+        return (31 * (31 + numerator.hashCode() * numS) + denominator.hashCode() * denS)
* numS * denS;
     }
 
     /**
diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java
b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java
index be85fb3..2ab8932 100644
--- a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java
+++ b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java
@@ -707,16 +707,15 @@ public final class Fraction
     @Override
     public int hashCode() {
         // Incorporate the sign and absolute values of the numerator and denominator.
-        // Equivalent to
-        // Arrays.hashCode(new int[] {signum(), Math.abs(numerator), Math.abs(denominator)})
+        // Equivalent to:
         // int hash = 1;
-        // hash = 31 * hash + signum();
         // hash = 31 * hash + Math.abs(numerator);
         // hash = 31 * hash + Math.abs(denominator);
-        // Note: x * Integer.signum(x) is equivalent to Math.abs(x).
+        // hash = hash * signum()
+        // Note: x * Integer.signum(x) == Math.abs(x).
         final int numS = Integer.signum(numerator);
         final int denS = Integer.signum(denominator);
-        return 31 * (31 * (31 + numS * denS) + numerator * numS) + denominator * denS;
+        return (31 * (31 + numerator * numS) + denominator * denS) * numS * denS;
     }
 
     /**
diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
index 4d3377c..0b7030d 100644
--- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
+++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
@@ -647,10 +647,10 @@ public class BigFractionTest {
         Assertions.assertNotSame(f1, f2, "Do not call this assertion with the same object");
         Assertions.assertEquals(f1, f2);
         Assertions.assertEquals(f1.hashCode(), f2.hashCode(), "Equal fractions have different
hashCode");
-        // Check the hashcode computation.
+        // Check the computation matches the result of Arrays.hashCode and the signum.
         // This is not mandated but is a recommendation.
-        final int expected = Arrays.hashCode(new Object[] {f1.signum(),
-                                                           f1.getNumerator().abs(),
+        final int expected = f1.signum() *
+                             Arrays.hashCode(new Object[] {f1.getNumerator().abs(),
                                                            f1.getDenominator().abs()});
         Assertions.assertEquals(expected, f1.hashCode(), "Hashcode not equal to using Arrays.hashCode");
     }
diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
index ada4269..4c5f847 100644
--- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
+++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/FractionTest.java
@@ -490,10 +490,10 @@ public class FractionTest {
         Assertions.assertNotSame(f1, f2, "Do not call this assertion with the same object");
         Assertions.assertEquals(f1, f2);
         Assertions.assertEquals(f1.hashCode(), f2.hashCode(), "Equal fractions have different
hashCode");
-        // Check the hashcode computation.
+        // Check the computation matches the result of Arrays.hashCode and the signum.
         // This is not mandated but is a recommendation.
-        final int expected = Arrays.hashCode(new int[] {f1.signum(),
-                                                        Math.abs(f1.getNumerator()),
+        final int expected = f1.signum() *
+                             Arrays.hashCode(new int[] {Math.abs(f1.getNumerator()),
                                                         Math.abs(f1.getDenominator())});
         Assertions.assertEquals(expected, f1.hashCode(), "Hashcode not equal to using Arrays.hashCode");
     }


Mime
View raw message