Fix PrimitiveHashSet#iterator()#remove() to not rehash. Fixes #481

Signed-off-by: Nikhil Nanivadekar <nikhil.nanivadekar@gs.com>
eclipse · Mar 19, 2018 · cc6a292 · cc6a292
1 parent cb845a2
commit cc6a292
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/eclipse-collections-code-generator/src/main/resources/impl/set/mutable/primitiveHashSet.stg b/eclipse-collections-code-generator/src/main/resources/impl/set/mutable/primitiveHashSet.stg
@@ -294,6 +294,11 @@ public class <name>HashSet extends Abstract<name>Set implements Mutable<name>Set
 
     @Override
     public boolean remove(<type> value)
+    {
+        return this.remove(value, true);
+    }
+
+    private boolean remove(<type> value, boolean rehash)
     {
         if (isBetweenZeroAndThirtyOne(value))
         {
@@ -316,7 +321,7 @@ public class <name>HashSet extends Abstract<name>Set implements Mutable<name>Set
             this.table[index] = REMOVED;
             this.occupiedWithData--;
             this.occupiedWithSentinels++;
-            if (this.occupiedWithSentinels > this.maxOccupiedWithSentinels())
+            if (rehash && this.occupiedWithSentinels > this.maxOccupiedWithSentinels())
             {
                 this.rehash();
             }
@@ -1610,7 +1615,7 @@ public class <name>HashSet extends Abstract<name>Set implements Mutable<name>Set
             {
                 removeValue = <name>HashSet.this.table[this.position - 1];
             }
-            <name>HashSet.this.remove(removeValue);
+            <name>HashSet.this.remove(removeValue, false);
             this.count--;
         }
     }

diff --git a/...e-collections-code-generator/src/main/resources/test/set/mutable/primitiveHashSetTest.stg b/...e-collections-code-generator/src/main/resources/test/set/mutable/primitiveHashSetTest.stg
@@ -8,16 +8,18 @@ targetPath() ::= "org/eclipse/collections/impl/set/mutable/primitive"
 fileName(primitive) ::= "<primitive.name>HashSetTest"
 
 class(primitive) ::= <<
-<body(primitive.type, primitive.wrapperName, primitive.name)>
+<body(primitive.type, primitive.wrapperName, primitive.name, primitive.charPrimitive, primitive.shortPrimitive)>
 >>
 
-body(type, wrapperName, name) ::= <<
+body(type, wrapperName, name, charPrimitive, shortPrimitive) ::= <<
 <copyright()>
 
 package org.eclipse.collections.impl.set.mutable.primitive;
 
 import java.lang.reflect.Field;
 
+import org.eclipse.collections.api.iterator.Mutable<name>Iterator;
+import org.eclipse.collections.api.set.primitive.Mutable<name>Set;
 import org.eclipse.collections.impl.factory.primitive.<name>Sets;
 import org.eclipse.collections.impl.list.mutable.primitive.<name>ArrayList;
 import org.eclipse.collections.impl.test.Verify;
@@ -177,6 +179,27 @@ public class <name>HashSetTest extends Abstract<name>SetTestCase
         Assert.assertEquals(0, occupiedWithSentinels.get(hashSet));
     }
 
+    @Test
+    public void iterator_remove()
+    {
+        Mutable<name>Set set = <name>Sets.mutable.empty();
+
+        int <if(shortPrimitive)>max = 65_536<elseif(charPrimitive)>max = 10<else>max = 100_000<endif>;
+        for (Integer i = 0; i \< max; i++)
+        {
+            <if(charPrimitive)>set.add(i.toString().charAt(0));<else>set.add(i.<type>Value());<endif>
+        }
+
+        Mutable<name>Iterator iterator = set.<type>Iterator();
+        Verify.assertSize(max, set);
+        while (iterator.hasNext())
+        {
+            iterator.next();
+            iterator.remove();
+            Verify.assertSize(--max, set);
+        }
+    }
+
     @Test
     public void addEverySlot()
     {