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-geometry.git
commit 318e627fb0483e29d4680a585715b75dd6181048 Author: Alex Herbert <aherb...@apache.org> AuthorDate: Fri Mar 28 16:07:35 2025 +0000 Fix hash code collision for Vector2D and Vector3D This commit modifies the `hashCode()` method for `Vector2D` and the `Vector3D` classes in order to ensure unique values are generated for non-identical instances of vectors. --- .../geometry/euclidean/threed/Vector3D.java | 6 +++- .../commons/geometry/euclidean/twod/Vector2D.java | 5 ++- .../geometry/euclidean/threed/Vector3DTest.java | 39 ++++++++++++++++++++++ .../geometry/euclidean/twod/Vector2DTest.java | 35 ++++++++++++++++++- .../geometry/spherical/twod/GreatCircleTest.java | 4 ++- 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java index 15ed75f1..991d91b5 100644 --- a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java +++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java @@ -408,7 +408,11 @@ public class Vector3D extends MultiDimensionalEuclideanVector<Vector3D> { if (isNaN()) { return 642; } - return 643 * (164 * Double.hashCode(x) + 3 * Double.hashCode(y) + Double.hashCode(z)); + int result = 1; + result = 31 * result + Double.hashCode(x); + result = 31 * result + Double.hashCode(y); + result = 31 * result + Double.hashCode(z); + return result; } /** diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/twod/Vector2D.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/twod/Vector2D.java index 738bbf75..93de0950 100644 --- a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/twod/Vector2D.java +++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/twod/Vector2D.java @@ -348,7 +348,10 @@ public class Vector2D extends MultiDimensionalEuclideanVector<Vector2D> { if (isNaN()) { return 542; } - return 122 * (76 * Double.hashCode(x) + Double.hashCode(y)); + int result = 1; + result = 31 * result + Double.hashCode(x); + result = 31 * result + Double.hashCode(y); + return result; } /** diff --git a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java index 4d7c435f..8630b68e 100644 --- a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java +++ b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java @@ -32,6 +32,9 @@ import org.apache.commons.rng.UniformRandomProvider; import org.apache.commons.rng.simple.RandomSource; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; class Vector3DTest { @@ -1146,6 +1149,42 @@ class Vector3DTest { Assertions.assertEquals(b.hashCode(), d.hashCode()); } + @ParameterizedTest + @ValueSource(doubles = {1.23, 4.56, 42, Math.PI}) + void testHashCodeCollisions_symmetricAboutZero(double any) { + testHashCodeCollisions_symmetricAboutArbitraryValue(any, 0.0); + } + + @ParameterizedTest + @CsvSource({ + "1.23, 0.5", + "4.56, 0.25", + "42, -16", + "3.141592, -9.87", + }) + void testHashCodeCollisions_symmetricAboutArbitraryValue(double any, double arb) { + final double neg = -any; + final double pos = +any; + + final Vector3D negX = Vector3D.of(neg, arb, arb); + final Vector3D posX = Vector3D.of(pos, arb, arb); + final Vector3D negY = Vector3D.of(arb, neg, arb); + final Vector3D posY = Vector3D.of(arb, pos, arb); + final Vector3D negZ = Vector3D.of(arb, arb, neg); + final Vector3D posZ = Vector3D.of(arb, arb, pos); + + int xNegHash = negX.hashCode(); + int xPosHash = posX.hashCode(); + int yNegHash = negY.hashCode(); + int yPosHash = posY.hashCode(); + int zNegHash = negZ.hashCode(); + int zPosHash = posZ.hashCode(); + + Assertions.assertNotEquals(xNegHash, xPosHash, "+/- x hash"); + Assertions.assertNotEquals(yNegHash, yPosHash, "+/- y hash"); + Assertions.assertNotEquals(zNegHash, zPosHash, "+/- z hash"); + } + @Test void testToString() { // arrange diff --git a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/twod/Vector2DTest.java b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/twod/Vector2DTest.java index 72eb3ae1..ad0803e9 100644 --- a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/twod/Vector2DTest.java +++ b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/twod/Vector2DTest.java @@ -29,6 +29,9 @@ import org.apache.commons.numbers.angle.Angle; import org.apache.commons.numbers.core.Precision; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; class Vector2DTest { @@ -588,7 +591,6 @@ class Vector2DTest { Assertions.assertEquals(0.004999958333958323, Vector2D.of(20.0, 0.0).angle(Vector2D.of(20.0, 0.1)), EPS); } - @Test void testAngle_illegalNorm() { // arrange @@ -912,6 +914,37 @@ class Vector2DTest { Assertions.assertEquals(Vector2D.of(0, Double.NaN).hashCode(), Vector2D.of(Double.NaN, 0).hashCode()); } + @ParameterizedTest + @ValueSource(doubles = {1.23, 4.56, 42, Math.PI}) + void testHashCodeCollisions_symmetricAboutZero(double any) { + testHashCodeCollisions_symmetricAboutArbitraryValue(any, 0.0); + } + + @ParameterizedTest + @CsvSource({ + "1.23, 0.5", + "4.56, 0.25", + "42, -16", + "3.141592, -9.87", + }) + void testHashCodeCollisions_symmetricAboutArbitraryValue(double any, double arb) { + final double neg = -any; + final double pos = +any; + + final Vector2D negX = Vector2D.of(neg, arb); + final Vector2D posX = Vector2D.of(pos, arb); + final Vector2D negY = Vector2D.of(arb, neg); + final Vector2D posY = Vector2D.of(arb, pos); + + int xNegHash = negX.hashCode(); + int xPosHash = posX.hashCode(); + int yNegHash = negY.hashCode(); + int yPosHash = posY.hashCode(); + + Assertions.assertNotEquals(xNegHash, xPosHash, "+/- x hash"); + Assertions.assertNotEquals(yNegHash, yPosHash, "+/- y hash"); + } + @Test void testEquals() { // arrange diff --git a/commons-geometry-spherical/src/test/java/org/apache/commons/geometry/spherical/twod/GreatCircleTest.java b/commons-geometry-spherical/src/test/java/org/apache/commons/geometry/spherical/twod/GreatCircleTest.java index d4aaa388..6b60fd02 100644 --- a/commons-geometry-spherical/src/test/java/org/apache/commons/geometry/spherical/twod/GreatCircleTest.java +++ b/commons-geometry-spherical/src/test/java/org/apache/commons/geometry/spherical/twod/GreatCircleTest.java @@ -664,7 +664,9 @@ class GreatCircleTest { Assertions.assertEquals(hash, a.hashCode()); Assertions.assertNotEquals(hash, b.hashCode()); - Assertions.assertNotEquals(hash, c.hashCode()); + // Equal objects have equal hash codes + Assertions.assertEquals(a, c); + Assertions.assertEquals(hash, c.hashCode()); Assertions.assertNotEquals(hash, d.hashCode()); Assertions.assertEquals(hash, e.hashCode());