Merge pull request #121 from nitsanw/hashset-equals

[Java] Make hashset (int and object) Set::equals compatible
real-logic · Dec 18, 2017 · 34d25cc · 34d25cc
2 parents 7b72974 + 814a5bf
commit 34d25cc
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 59 deletions.
diff --git a/agrona/src/main/java/org/agrona/collections/IntHashSet.java b/agrona/src/main/java/org/agrona/collections/IntHashSet.java
@@ -652,7 +652,29 @@ public boolean equals(final Object other)
                    containsAll(otherSet);
         }
 
-        return false;
+        if (!(other instanceof Set))
+        {
+            return false;
+        }
+
+        final Set<?> c = (Set<?>)other;
+        if (c.size() != size())
+        {
+            return false;
+        }
+
+        try
+        {
+            return containsAll(c);
+        }
+        catch (final ClassCastException unused)
+        {
+            return false;
+        }
+        catch (final @DoNotSub NullPointerException unused)
+        {
+            return false;
+        }
     }
 
     /**

diff --git a/agrona/src/main/java/org/agrona/collections/ObjectHashSet.java b/agrona/src/main/java/org/agrona/collections/ObjectHashSet.java
@@ -566,7 +566,29 @@ public boolean equals(final Object other)
             return otherSet.size == size && containsAll(otherSet);
         }
 
-        return false;
+        if (!(other instanceof Set))
+        {
+            return false;
+        }
+
+        final Set<?> c = (Set<?>)other;
+        if (c.size() != size())
+        {
+            return false;
+        }
+
+        try
+        {
+            return containsAll(c);
+        }
+        catch (final ClassCastException unused)
+        {
+            return false;
+        }
+        catch (final NullPointerException unused)
+        {
+            return false;
+        }
     }
 
     /**

diff --git a/agrona/src/test/java/org/agrona/collections/IntHashSetTest.java b/agrona/src/test/java/org/agrona/collections/IntHashSetTest.java
@@ -626,63 +626,6 @@ public void removeElementsFromIterator()
         assertThat(testSet, hasSize(1));
     }
 
-    private static void addTwoElements(final IntHashSet obj)
-    {
-        obj.add(1);
-        obj.add(1001);
-    }
-
-    private static void addTwoElements(final HashSet<Integer> obj)
-    {
-        obj.add(1);
-        obj.add(1001);
-    }
-
-    private void assertIteratorHasElements()
-    {
-        final Iterator<Integer> iter = testSet.iterator();
-
-        final Set<Integer> values = new HashSet<>();
-
-        assertTrue(iter.hasNext());
-        values.add(iter.next());
-        assertTrue(iter.hasNext());
-        values.add(iter.next());
-        assertFalse(iter.hasNext());
-
-        assertContainsElements(values);
-    }
-
-    private void assertIteratorHasElementsWithoutHasNext()
-    {
-        final Iterator<Integer> iter = testSet.iterator();
-
-        final Set<Integer> values = new HashSet<>();
-
-        values.add(iter.next());
-        values.add(iter.next());
-
-        assertContainsElements(values);
-    }
-
-    private static void assertArrayContainingElements(final Integer[] result)
-    {
-        assertThat(result, arrayContainingInAnyOrder(1, 1001));
-    }
-
-    private static void assertContainsElements(final Set<Integer> other)
-    {
-        assertThat(other, containsInAnyOrder(1, 1001));
-    }
-
-    private void exhaustIterator()
-    {
-        final IntIterator iterator = testSet.iterator();
-        iterator.next();
-        iterator.next();
-        iterator.next();
-    }
-
     @Test
     public void shouldNotContainMissingValueInitially()
     {
@@ -861,4 +804,84 @@ public void shouldRemoveMissingValueWhenCleared()
 
         assertFalse(testSet.contains(MISSING_VALUE));
     }
+
+    @Test
+    public void shouldHaveCompatibleEqualsAndHashcode()
+    {
+        final HashSet compatibleSet = new HashSet();
+        final long seed = System.nanoTime();
+        final Random r = new Random(seed);
+        for (int i = 0; i < 1024; i++)
+        {
+            final int value = r.nextInt();
+            compatibleSet.add(value);
+            testSet.add(value);
+        }
+
+        if (r.nextBoolean())
+        {
+            compatibleSet.add(MISSING_VALUE);
+            testSet.add(MISSING_VALUE);
+        }
+        assertEquals("Fail with seed:" + seed, testSet, compatibleSet);
+        assertEquals("Fail with seed:" + seed, compatibleSet, testSet);
+        assertEquals("Fail with seed:" + seed, compatibleSet.hashCode(), testSet.hashCode());
+    }
+
+    private static void addTwoElements(final IntHashSet obj)
+    {
+        obj.add(1);
+        obj.add(1001);
+    }
+
+    private static void addTwoElements(final HashSet<Integer> obj)
+    {
+        obj.add(1);
+        obj.add(1001);
+    }
+
+    private void assertIteratorHasElements()
+    {
+        final Iterator<Integer> iter = testSet.iterator();
+
+        final Set<Integer> values = new HashSet<>();
+
+        assertTrue(iter.hasNext());
+        values.add(iter.next());
+        assertTrue(iter.hasNext());
+        values.add(iter.next());
+        assertFalse(iter.hasNext());
+
+        assertContainsElements(values);
+    }
+
+    private void assertIteratorHasElementsWithoutHasNext()
+    {
+        final Iterator<Integer> iter = testSet.iterator();
+
+        final Set<Integer> values = new HashSet<>();
+
+        values.add(iter.next());
+        values.add(iter.next());
+
+        assertContainsElements(values);
+    }
+
+    private static void assertArrayContainingElements(final Integer[] result)
+    {
+        assertThat(result, arrayContainingInAnyOrder(1, 1001));
+    }
+
+    private static void assertContainsElements(final Set<Integer> other)
+    {
+        assertThat(other, containsInAnyOrder(1, 1001));
+    }
+
+    private void exhaustIterator()
+    {
+        final IntIterator iterator = testSet.iterator();
+        iterator.next();
+        iterator.next();
+        iterator.next();
+    }
 }
diff --git a/agrona/src/test/java/org/agrona/collections/ObjectHashSetIntegerTest.java b/agrona/src/test/java/org/agrona/collections/ObjectHashSetIntegerTest.java
@@ -615,6 +615,24 @@ public void shouldIterateOverExpandedSet()
         assertFalse(iter.hasNext());
     }
 
+    @Test
+    public void shouldHaveCompatibleEqualsAndHashcode()
+    {
+        final HashSet compatibleSet = new HashSet();
+        final long seed = System.nanoTime();
+        final Random r = new Random(seed);
+        for (int i = 0; i < 1024; i++)
+        {
+            final int value = r.nextInt();
+            compatibleSet.add(value);
+            testSet.add(value);
+        }
+
+        assertEquals("Fail with seed:" + seed, testSet, compatibleSet);
+        assertEquals("Fail with seed:" + seed, compatibleSet, testSet);
+        assertEquals("Fail with seed:" + seed, compatibleSet.hashCode(), testSet.hashCode());
+    }
+
     private static void addTwoElements(final ObjectHashSet<Integer> obj)
     {
         obj.add(1);