Fix bugs in handling key collision and keyset iterator based remove o…

…peration in Primitive Maps with Hashing Strategy. Signed-off-by: Ranjan Jajodia <ranjan.jajodia@gs.com>
eclipse · Feb 19, 2016 · 4cd64a5 · 4cd64a5
1 parent 612f0df
commit 4cd64a5
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 11 deletions.
diff --git a/eclipse-collections-code-generator/src/main/resources/impl/bag/mutable/primitiveHashBag.stg b/eclipse-collections-code-generator/src/main/resources/impl/bag/mutable/primitiveHashBag.stg
@@ -382,7 +382,7 @@ public final class <name>HashBag extends Abstract<name>Iterable implements Mutab
         return true;
     }
 
-    public void forEach(final <name>Procedure procedure)
+    public void forEach(<name>Procedure procedure)
     {
         this.each(procedure);
     }

diff --git a/...collections-code-generator/src/main/resources/impl/map/mutable/objectPrimitiveHashMap.stg b/...collections-code-generator/src/main/resources/impl/map/mutable/objectPrimitiveHashMap.stg
@@ -1448,7 +1448,7 @@ public class Object<name>HashMap\<K> implements MutableObject<name>Map\<K>, Exte
             if (isNonSentinel(this.currentKey))
             {
                 int index = this.position - 1;
-                Object<name>HashMap.this.removeKeyAtIndex(toNonSentinel(this.currentKey), index);
+                Object<name>HashMap.this.removeKeyAtIndex(Object<name>HashMap.this.toNonSentinel(this.currentKey), index);
             }
             else
             {

diff --git a/...nerator/src/main/resources/impl/map/mutable/objectPrimitiveHashMapWithHashingStrategy.stg b/...nerator/src/main/resources/impl/map/mutable/objectPrimitiveHashMapWithHashingStrategy.stg
@@ -178,7 +178,6 @@ public class Object<name>HashMapWithHashingStrategy\<K> implements MutableObject
         return new Object<name>HashMapWithHashingStrategy\<K>(map.hashingStrategy, map);
     }
 
-
     public static \<K> Object<name>HashMapWithHashingStrategy\<K> newWithKeysValues(HashingStrategy\<? super K> hashingStrategy, K key1, <type> value1)
     {
         return new Object<name>HashMapWithHashingStrategy\<K>(hashingStrategy, 1).withKeyValue(key1, value1);
@@ -1176,9 +1175,12 @@ public class Object<name>HashMapWithHashingStrategy\<K> implements MutableObject
 
     private boolean nullSafeEquals(K value, Object other)
     {
-        if (value == null && other == null)
+        if (value == null)
         {
-            return true;
+            if (other == null)
+            {
+                return true;
+            }
         }
         else if (value != NULL_KEY && other != null)
         {
@@ -1469,7 +1471,7 @@ public class Object<name>HashMapWithHashingStrategy\<K> implements MutableObject
             if (isNonSentinel(this.currentKey))
             {
                 int index = this.position - 1;
-                Object<name>HashMapWithHashingStrategy.this.removeKeyAtIndex(this.currentKey, index);
+                Object<name>HashMapWithHashingStrategy.this.removeKeyAtIndex(Object<name>HashMapWithHashingStrategy.this.toNonSentinel(this.currentKey), index);
             }
             else
             {

diff --git a/...tor/src/main/resources/test/map/mutable/objectPrimitiveHashMapWithHashingStrategyTest.stg b/...tor/src/main/resources/test/map/mutable/objectPrimitiveHashMapWithHashingStrategyTest.stg
@@ -76,6 +76,19 @@ public class Object<name>HashMapWithHashingStrategyTest extends Object<name>Hash
 
     private static final HashingStrategy\<Person> FIRST_NAME_HASHING_STRATEGY = HashingStrategies.fromFunction(Person.TO_FIRST);
     private static final HashingStrategy\<Person> LAST_NAME_HASHING_STRATEGY = HashingStrategies.fromFunction(Person.TO_LAST);
+    private static final HashingStrategy\<Person> CONSTANT_HASHCODE_STRATEGY = new HashingStrategy\<Person>() {
+       @Override
+       public int computeHashCode(Person object)
+       {
+           return 0;
+       }
+
+       @Override
+       public boolean equals(Person person1, Person person2)
+       {
+           return person1.getLastName().equals(person2.getLastName());
+       }
+    };
 
     private static final Person JOHNSMITH = new Person("John", "Smith");
     private static final Person JANESMITH = new Person("Jane", "Smith");
@@ -213,10 +226,15 @@ public class Object<name>HashMapWithHashingStrategyTest extends Object<name>Hash
     {
         Object<name>HashMapWithHashingStrategy\<Person> map1 = Object<name>HashMapWithHashingStrategy.newWithKeysValues(LAST_NAME_HASHING_STRATEGY, JOHNDOE, <(literal.(type))("1")>, JANEDOE, <(literal.(type))("1")>, JOHNSMITH, <(literal.(type))("1")>, JANESMITH, <(literal.(type))("1")>);
         Object<name>HashMapWithHashingStrategy\<Person> map2 = Object<name>HashMapWithHashingStrategy.newWithKeysValues(FIRST_NAME_HASHING_STRATEGY, JOHNDOE, <(literal.(type))("1")>, JANEDOE, <(literal.(type))("1")>, JOHNSMITH, <(literal.(type))("1")>, JANESMITH, <(literal.(type))("1")>);
+        Object<name>HashMapWithHashingStrategy\<Person> mapWithConstantHashCodeStrategy = Object<name>HashMapWithHashingStrategy.newWithKeysValues(CONSTANT_HASHCODE_STRATEGY, JOHNDOE, <(literal.(type))("1")>, JANEDOE, <(literal.(type))("1")>, JOHNSMITH, <(literal.(type))("1")>, JANESMITH, <(literal.(type))("1")>);
 
         Assert.assertEquals(map1, map2);
         Assert.assertEquals(map2, map1);
+        Assert.assertEquals(mapWithConstantHashCodeStrategy, map2);
+        Assert.assertEquals(map2, mapWithConstantHashCodeStrategy);
         Assert.assertNotEquals(map1.hashCode(), map2.hashCode());
+        Assert.assertNotEquals(map1.hashCode(), mapWithConstantHashCodeStrategy.hashCode());
+        Assert.assertNotEquals(map2.hashCode(), mapWithConstantHashCodeStrategy.hashCode());
 
         Object<name>HashMapWithHashingStrategy\<Person> map3 = Object<name>HashMapWithHashingStrategy.newWithKeysValues(LAST_NAME_HASHING_STRATEGY, <allPeople()>);
 

diff --git a/...lipse/collections/impl/map/mutable/primitive/ObjectBooleanHashMapWithHashingStrategy.java b/...lipse/collections/impl/map/mutable/primitive/ObjectBooleanHashMapWithHashingStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015 Goldman Sachs.
+ * Copyright (c) 2016 Goldman Sachs.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * and Eclipse Distribution License v. 1.0 which accompany this distribution.
@@ -215,7 +215,6 @@ private int fastCeil(float v)
         return possibleResult;
     }
 
-
     @Override
     public boolean equals(Object obj)
     {
@@ -938,11 +937,14 @@ private static boolean isRemovedKey(Object key)
 
     private boolean nullSafeEquals(K key, Object other)
     {
-        if (key == null && other == null)
+        if (key == null)
         {
-            return true;
+            if (other == null)
+            {
+                return true;
+            }
         }
-        if (key != NULL_KEY && other != null)
+        else if (key != NULL_KEY && other != null)
         {
             if (this.hashingStrategy.equals(key, this.toNonSentinel(other)))
             {

diff --git a/...e/collections/impl/map/mutable/primitive/ObjectBooleanHashMapWithHashingStrategyTest.java b/...e/collections/impl/map/mutable/primitive/ObjectBooleanHashMapWithHashingStrategyTest.java
@@ -53,6 +53,19 @@ public boolean equals(Integer object1, Integer object2)
 
     private static final HashingStrategy<Person> FIRST_NAME_HASHING_STRATEGY = HashingStrategies.fromFunction(Person.TO_FIRST);
     private static final HashingStrategy<Person> LAST_NAME_HASHING_STRATEGY = HashingStrategies.fromFunction(Person.TO_LAST);
+    private static final HashingStrategy<Person> CONSTANT_HASHCODE_STRATEGY = new HashingStrategy<Person>() {
+        @Override
+        public int computeHashCode(Person object)
+        {
+            return 0;
+        }
+
+        @Override
+        public boolean equals(Person person1, Person person2)
+        {
+            return person1.getLastName().equals(person2.getLastName());
+        }
+    };
 
     private static final Person JOHNSMITH = new Person("John", "Smith");
     private static final Person JANESMITH = new Person("Jane", "Smith");
@@ -185,10 +198,15 @@ public void equals_with_hashing_strategy()
     {
         ObjectBooleanHashMapWithHashingStrategy<Person> map1 = ObjectBooleanHashMapWithHashingStrategy.newWithKeysValues(LAST_NAME_HASHING_STRATEGY, JOHNDOE, true, JANEDOE, true, JOHNSMITH, true, JANESMITH, true);
         ObjectBooleanHashMapWithHashingStrategy<Person> map2 = ObjectBooleanHashMapWithHashingStrategy.newWithKeysValues(FIRST_NAME_HASHING_STRATEGY, JOHNDOE, true, JANEDOE, true, JOHNSMITH, true, JANESMITH, true);
+        ObjectBooleanHashMapWithHashingStrategy<Person> mapWithConstantHashCodeStrategy = ObjectBooleanHashMapWithHashingStrategy.newWithKeysValues(CONSTANT_HASHCODE_STRATEGY, JOHNDOE, true, JANEDOE, true, JOHNSMITH, true, JANESMITH, true);
 
         Assert.assertEquals(map1, map2);
         Assert.assertEquals(map2, map1);
+        Assert.assertEquals(mapWithConstantHashCodeStrategy, map2);
+        Assert.assertEquals(map2, mapWithConstantHashCodeStrategy);
         Assert.assertNotEquals(map1.hashCode(), map2.hashCode());
+        Assert.assertNotEquals(map1.hashCode(), mapWithConstantHashCodeStrategy.hashCode());
+        Assert.assertNotEquals(map2.hashCode(), mapWithConstantHashCodeStrategy.hashCode());
 
         ObjectBooleanHashMapWithHashingStrategy<Person> map3 = ObjectBooleanHashMapWithHashingStrategy.newWithKeysValues(LAST_NAME_HASHING_STRATEGY, JOHNDOE, true, JANEDOE, false, JOHNSMITH, true, JANESMITH, false);