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());

Reply via email to