From 814a5bfd5d5b546bf63613ad6ce99f5120f39183 Mon Sep 17 00:00:00 2001 From: nitsanw Date: Mon, 18 Dec 2017 09:53:42 +0200 Subject: [PATCH] [Java] Make hashset equals compatible --- .../org/agrona/collections/IntHashSet.java | 24 ++- .../org/agrona/collections/ObjectHashSet.java | 24 ++- .../agrona/collections/IntHashSetTest.java | 137 ++++++++++-------- .../collections/ObjectHashSetIntegerTest.java | 18 +++ 4 files changed, 144 insertions(+), 59 deletions(-) diff --git a/agrona/src/main/java/org/agrona/collections/IntHashSet.java b/agrona/src/main/java/org/agrona/collections/IntHashSet.java index c29d0af1e..552dc4be1 100644 --- 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 index 5917d0075..26654871e 100644 --- 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 index 4ac0d2eab..23ecadfc8 100644 --- 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 obj) - { - obj.add(1); - obj.add(1001); - } - - private void assertIteratorHasElements() - { - final Iterator iter = testSet.iterator(); - - final Set 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 iter = testSet.iterator(); - - final Set 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 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 obj) + { + obj.add(1); + obj.add(1001); + } + + private void assertIteratorHasElements() + { + final Iterator iter = testSet.iterator(); + + final Set 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 iter = testSet.iterator(); + + final Set 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 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 index 43ce0a915..acded0a2e 100644 --- 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 obj) { obj.add(1);