Skip to content

Commit 94f612b

Browse files
committed
[LANG-1764] Several hash collisions in Fraction class
1 parent 7d419b9 commit 94f612b

3 files changed

Lines changed: 28 additions & 13 deletions

File tree

src/changes/changes.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ The <action> type attribute can be add,update,fix,remove.
8181
<action type="fix" dev="ggregory" due-to="Gary Gregory">Don't call TypeUtils.toString(Type) on every array item in TypeUtils.parameterize[WithOwner](Type, Class, Map, Type>) unless required.</action>
8282
<action type="fix" dev="ggregory" due-to="Gary Gregory">Remove -nouses directive from maven-bundle-plugin. OSGi package imports now state 'uses' definitions for package imports, this doesn't affect JPMS (from org.apache.commons:commons-parent:80).</action>
8383
<action type="fix" dev="ggregory" due-to="Gary Gregory">Instead of throwing a NullPointerException, ArrayUtils.toStringArray(Object[]) should return "null" for null elements like ArrayUtils.toStringArray(Object[], String) returns its valueForNullElements.</action>
84-
<action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate NumericEntityUnescaper.OPTION in favor of Apache Commons Text.</action>
84+
<action issue="LANG-1764" type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate NumericEntityUnescaper.OPTION in favor of Apache Commons Text.</action>
85+
<action type="fix" dev="ggregory" due-to="Gary Gregory">Several hash collisions in Fraction class.</action>
8586
<!-- ADD -->
8687
<action type="add" dev="ggregory" due-to="Gary Gregory">Add Strings and refactor StringUtils.</action>
8788
<action issue="LANG-1747" type="add" dev="ggregory" due-to="Oliver B. Fischer, Gary Gregory">Add StopWatch.run([Failable]Runnable) and get([Failable]Supplier).</action>

src/main/java/org/apache/commons/lang3/math/Fraction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ public int getProperWhole() {
703703
public int hashCode() {
704704
if (hashCode == 0) {
705705
// hash code update should be atomic.
706-
hashCode = 37 * (37 * 17 + getNumerator()) + getDenominator();
706+
hashCode = Objects.hash(denominator, numerator);
707707
}
708708
return hashCode;
709709
}

src/test/java/org/apache/commons/lang3/math/FractionTest.java

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626

2727
import org.apache.commons.lang3.AbstractLangTest;
2828
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.params.ParameterizedTest;
30+
import org.junit.jupiter.params.provider.CsvSource;
2931

3032
/**
3133
* Test cases for the {@link Fraction} class
@@ -616,17 +618,29 @@ public void testHashCode() {
616618
assertTrue(f1.hashCode() != f2.hashCode());
617619
f2 = Fraction.getFraction(6, 10);
618620
assertTrue(f1.hashCode() != f2.hashCode());
619-
// Use cases from https://issues.apache.org/jira/browse/LANG-1764
620-
assertNotEquals(Fraction.getFraction(0, 37), Fraction.getFraction(-464320789, 46));
621-
assertNotEquals(Fraction.getFraction(0, 37), Fraction.getFraction(-464320788, 9));
622-
assertNotEquals(Fraction.getFraction(0, 37), Fraction.getFraction(1857283155, 38));
623-
assertNotEquals(Fraction.getFraction(0, 25185704), Fraction.getFraction(1161454280, 1050304));
624-
assertNotEquals(Fraction.getFraction(0, 38817068), Fraction.getFraction(1509581512, 18875972));
625-
assertNotEquals(Fraction.getFraction(0, 38817068), Fraction.getFraction(-2146369536, 2145078572));
626-
assertNotEquals(Fraction.getFraction(1400217380, 128), Fraction.getFraction(2092630052, 150535040));
627-
assertNotEquals(Fraction.getFraction(1400217380, 128), Fraction.getFraction(-580400986, 268435638));
628-
assertNotEquals(Fraction.getFraction(1400217380, 2147483592), Fraction.getFraction(-2147483648, 268435452));
629-
assertNotEquals(Fraction.getFraction(1756395909, 4194598), Fraction.getFraction(1174949894, 42860673));
621+
}
622+
623+
/**
624+
* Tests https://issues.apache.org/jira/browse/LANG-1764
625+
*/
626+
@ParameterizedTest
627+
// @formatter:off
628+
@CsvSource({
629+
"0, 37, -464320789, 46",
630+
"0, 37, -464320788, 9",
631+
"0, 37, 1857283155, 38",
632+
"0, 25185704, 1161454280, 1050304",
633+
"0, 38817068, 1509581512, 18875972",
634+
"0, 38817068, -2146369536, 2145078572",
635+
"1400217380, 128, 2092630052, 150535040",
636+
"1400217380, 128, -580400986, 268435638",
637+
"1400217380, 2147483592, -2147483648, 268435452",
638+
"1756395909, 4194598, 1174949894, 42860673"
639+
})
640+
// @formatter:on
641+
public void testHashCodeNotEquals(int f1n, int f1d, int f2n, int f2d) {
642+
assertNotEquals(Fraction.getFraction(f1n, f1d), Fraction.getFraction(f2n, f2d));
643+
assertNotEquals(Fraction.getFraction(f1n, f1d).hashCode(), Fraction.getFraction(f2n, f2d).hashCode());
630644
}
631645

632646
@Test

0 commit comments

Comments
 (0)