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 <[email protected]> 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"); }
