You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ah...@apache.org on 2020/04/08 21:17:19 UTC
[commons-numbers] 01/08: Clean-up Fraction hashcode.
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 <ah...@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");
}