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 2025/03/28 16:13:41 UTC
(commons-geometry) 01/02: Fix hash code collision for Vector2D and Vector3D
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 <ah...@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());