Tweak BadMap, add tests

config/src/main/java/com/typesafe/config/impl/BadMap.java

@@ -29,13 +29,13 @@ else if (next != null)
         }
     }
 
-    final int size;
-    final Entry[] entries;
+    private final int size;
+    private final Entry[] entries;
 
     private final static Entry[] emptyEntries = {};
 
     BadMap() {
-        this(0, (Entry[]) emptyEntries);
+        this(0, emptyEntries);
     }
 
     private BadMap(int size, Entry[] entries) {
@@ -51,19 +51,20 @@ BadMap<K,V> copyingPut(K k, V v) {
             // we passed in, so this block may not actually change
             // the entries size. the "-1" is to ensure we use
             // array length 2 when going from 0 to 1.
-            newEntries = new Entry[nextPrime((newSize*2) - 1)];
+            newEntries = new Entry[nextPrime((newSize * 2) - 1)];
         } else {
             newEntries = new Entry[entries.length];
         }
 
-        if (newEntries.length == entries.length)
+        if (newEntries.length == entries.length) {
             System.arraycopy(entries, 0, newEntries, 0, entries.length);
-        else
+        } else {
             rehash(entries, newEntries);
+        }
 
         int hash = Math.abs(k.hashCode());
         store(newEntries, hash, k, v);
-        return new BadMap<K,V>(newSize, newEntries);
+        return new BadMap<>(newSize, newEntries);
     }
 
     private static <K,V> void store(Entry[] entries, int hash, K k, V v) {
@@ -72,7 +73,7 @@ private static <K,V> void store(Entry[] entries, int hash, K k, V v) {
         entries[i] = new Entry(hash, k, v, old);
     }
 
-    private static <K,V> void store(Entry[] entries, Entry e) {
+    private static void store(Entry[] entries, Entry e) {
         int i = e.hash % entries.length;
         Entry old = entries[i]; // old may be null
         if (old == null && e.next == null) {
@@ -84,15 +85,12 @@ private static <K,V> void store(Entry[] entries, Entry e) {
         }
     }
 
-    private static <K,V> void rehash(Entry[] src, Entry[] dest) {
-        for (Entry old : src) {
-            if (old != null) {
-                if (old.next == null) {
-                    store(dest, old);
-                } else {
-                    store(dest, old.hash, old.key, old.value);
-                    store(dest, old.next);
-                }
+    private static void rehash(Entry[] src, Entry[] dest) {
+        for (Entry entry : src) {
+            // have to store each "next" element individually; they may belong in different indices
+            while (entry != null) {
+                store(dest, entry);
+                entry = entry.next;
             }
         }
     }
@@ -128,7 +126,7 @@ V get(K k) {
         2053, 3079, 4057, 7103, 10949, 16069, 32609, 65867, 104729
     };
 
-    private final static int nextPrime(int i) {
+    private static int nextPrime(int i) {
         for (int p : primes) {
             if (p > i)
                 return p;

config/src/main/java/com/typesafe/config/impl/ResolveMemos.java

@@ -1,8 +1,5 @@
 package com.typesafe.config.impl;
 
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * This exists because we have to memoize resolved substitutions as we go
  * through the config tree; otherwise we could end up creating multiple copies
@@ -18,7 +15,7 @@ private ResolveMemos(BadMap<MemoKey, AbstractConfigValue> memos) {
     }
 
     ResolveMemos() {
-        this(new BadMap<MemoKey, AbstractConfigValue>());
+        this(new BadMap<>());
     }
 
     AbstractConfigValue get(MemoKey key) {

config/src/test/scala/com/typesafe/config/impl/BadMapTest.scala

@@ -0,0 +1,94 @@
+package com.typesafe.config.impl
+
+import org.junit.Assert._
+import org.junit.Test
+
+class BadMapTest extends TestUtils {
+    @Test
+    def copyingPut(): Unit = {
+        val map = new BadMap[String, String]()
+        val copy = map.copyingPut("key", "value")
+
+        assertNull(map.get("key"))
+        assertEquals("value", copy.get("key"))
+    }
+
+    @Test
+    def retrieveOldElement(): Unit = {
+        val map = new BadMap[String, String]()
+            .copyingPut("key1", "value1")
+            .copyingPut("key2", "value2")
+            .copyingPut("key3", "value3")
+
+        assertEquals("value1", map.get("key1"))
+        assertEquals("value2", map.get("key2"))
+        assertEquals("value3", map.get("key3"))
+    }
+
+    @Test
+    def putOverride(): Unit = {
+        val map = new BadMap[String, String]()
+            .copyingPut("key", "value1")
+            .copyingPut("key", "value2")
+            .copyingPut("key", "value3")
+
+        assertEquals("value3", map.get("key"))
+    }
+
+    @Test
+    def notFound(): Unit = {
+        val map = new BadMap[String, String]()
+
+        assertNull(map.get("invalid key"))
+    }
+
+    @Test
+    def putMany(): Unit = {
+        val entries = (1 to 1000).map(i => (s"key$i", s"value$i"))
+        var map = new BadMap[String, String]()
+
+        for ((key, value) <- entries) {
+            map = map.copyingPut(key, value)
+        }
+
+        for ((key, value) <- entries) {
+            assertEquals(value, map.get(key))
+        }
+    }
+
+    @Test
+    def putSameHash(): Unit = {
+        val hash = 2
+        val entries = (1 to 10).map(i => (new UniqueKeyWithHash(hash), s"value$i"))
+        var map = new BadMap[UniqueKeyWithHash, String]()
+
+        for ((key, value) <- entries) {
+            map = map.copyingPut(key, value)
+        }
+
+        for ((key, value) <- entries) {
+            assertEquals(value, map.get(key))
+        }
+    }
+
+    @Test
+    def putSameHashModLength(): Unit = {
+        // given that the table will eventually be the following size, we insert entries who should
+        // eventually all share the same index and then later be redistributed once rehashed
+        val size = 11
+        val entries = (1 to size * 2).map(i => (new UniqueKeyWithHash(size * i), s"value$i"))
+        var map = new BadMap[UniqueKeyWithHash, String]()
+
+        for ((key, value) <- entries) {
+            map = map.copyingPut(key, value)
+        }
+
+        for ((key, value) <- entries) {
+            assertEquals(value, map.get(key))
+        }
+    }
+
+    private class UniqueKeyWithHash(hash: Int) {
+        override def hashCode(): Int = hash
+    }
+}