[Java] Fix containsValue in primitive maps to correctly handle missin…

…gValue
haiderny · Jan 14, 2017 · 56669f5 · 56669f5
1 parent 134ef92
commit 56669f5
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 18 deletions.
diff --git a/src/main/java/org/agrona/collections/Int2IntCounterMap.java b/src/main/java/org/agrona/collections/Int2IntCounterMap.java
@@ -330,19 +330,21 @@ public boolean containsKey(final int key)
      */
     public boolean containsValue(final int value)
     {
-        final int[] entries = this.entries;
-        @DoNotSub final int length = entries.length;
-
         boolean found = false;
-        for (@DoNotSub int i = 1; i < length; i += 2)
+        if (value != initialValue)
         {
-            if (value == entries[i])
+            final int[] entries = this.entries;
+            @DoNotSub final int length = entries.length;
+
+            for (@DoNotSub int i = 1; i < length; i += 2)
             {
-                found = true;
-                break;
+                if (value == entries[i])
+                {
+                    found = true;
+                    break;
+                }
             }
         }
-
         return found;
     }
 

diff --git a/src/main/java/org/agrona/collections/Int2IntHashMap.java b/src/main/java/org/agrona/collections/Int2IntHashMap.java
@@ -226,19 +226,21 @@ public boolean containsKey(final int key)
 
     public boolean containsValue(final int value)
     {
-        final int[] entries = this.entries;
-        @DoNotSub final int length = entries.length;
-
         boolean found = false;
-        for (@DoNotSub int valueIndex = 1; valueIndex < length; valueIndex += 2)
+        if (value != missingValue)
         {
-            if (value == entries[valueIndex])
+            final int[] entries = this.entries;
+            @DoNotSub final int length = entries.length;
+
+            for (@DoNotSub int valueIndex = 1; valueIndex < length; valueIndex += 2)
             {
-                found = true;
-                break;
+                if (value == entries[valueIndex])
+                {
+                    found = true;
+                    break;
+                }
             }
         }
-
         return found;
     }
 

diff --git a/src/test/java/org/agrona/collections/Int2IntCounterMapTest.java b/src/test/java/org/agrona/collections/Int2IntCounterMapTest.java
@@ -428,13 +428,13 @@ public void shouldRemoveEntryAfterIncToInitialVal()
         assertFalse(map.containsKey(1));
     }
     @Test(expected = IllegalArgumentException.class)
-    public void shouldNotAllowMissingValueAsValue()
+    public void shouldNotAllowInitialValueAsValue()
     {
         map.put(1, INITIAL_VALUE);
     }
 
     @Test
-    public void shouldAllowMissingValueAsKey()
+    public void shouldAllowInitialValueAsKey()
     {
         map.put(INITIAL_VALUE, 1);
 
@@ -457,4 +457,13 @@ public void shouldAllowMissingValueAsKey()
         assertEquals(0, map.size());
         assertEquals(INITIAL_VALUE, map.get(INITIAL_VALUE));
     }
+
+    @Test
+    public void shouldNotContainInitialValue()
+    {
+        assertFalse(map.containsValue(INITIAL_VALUE));
+        map.put(INITIAL_VALUE, 1);
+        assertFalse(map.containsValue(INITIAL_VALUE));
+    }
+
 }
diff --git a/src/test/java/org/agrona/collections/Int2IntHashMapTest.java b/src/test/java/org/agrona/collections/Int2IntHashMapTest.java
@@ -501,4 +501,12 @@ public void shouldAllowMissingValueAsKey()
         assertEquals(0, map.size());
         assertEquals(MISSING_VALUE, map.get(MISSING_VALUE));
     }
+
+    @Test
+    public void shouldNotContainMissingValue()
+    {
+        assertFalse(map.containsValue(MISSING_VALUE));
+        map.put(MISSING_VALUE, 1);
+        assertFalse(map.containsValue(MISSING_VALUE));
+    }
 }