Refactor Map-InternalMap to provide specialized linked string map and set

js/js.libraries/src/core/collections/HashMap.kt

@@ -100,26 +100,26 @@ open class HashMap<K, V> : AbstractMap<K, V> {
      */
     private val internalMap: InternalMap<K, V>
 
-    internal val equality: EqualityComparator
+    private val equality: EqualityComparator
 
 //    /**
 //     * A map of Strings onto values.
 //     */
 //    @Transient private var stringMap: InternalStringMap<K, V>? = null
 //
-    internal constructor(internalMapProvider: (HashMap<K, V>) -> InternalMap<K, V>, equality: EqualityComparator = EqualityComparator.HashCode) : super() {
+    internal constructor(internalMap: InternalMap<K, V>) : super() {
-        this.equality = equality
+        this.internalMap = internalMap
-        this.internalMap = internalMapProvider(this)
+        this.equality = internalMap.equality
     }
 
-    constructor() : this( { m -> InternalHashCodeMap(m) })
+    constructor() : this(InternalHashCodeMap(EqualityComparator.HashCode))
 //    init {
 //        reset()
 //    }
 
-    constructor(capacity: Int, loadFactor: Float = 0f) : this() {
+    constructor(initialCapacity: Int, loadFactor: Float = 0f) : this() {
         // This implementation of HashMap has no need of load factors or capacities.
-        require(capacity >= 0) { "Negative initial capacity" }
+        require(initialCapacity >= 0) { "Negative initial capacity" }
         require(loadFactor >= 0) { "Non-positive load factor" }
     }
 
@@ -273,5 +273,5 @@ open class HashMap<K, V> : AbstractMap<K, V> {
 
 
 public fun <V> stringMapOf(vararg pairs: Pair<String, V>): HashMap<String, V> {
-    return HashMap<String, V>({ m -> InternalStringMap(m) }).apply { putAll(pairs) }
+    return HashMap<String, V>(InternalStringMap(EqualityComparator.HashCode)).apply { putAll(pairs) }
 }

js/js.libraries/src/core/collections/InternalHashCodeMap.kt

@@ -35,14 +35,14 @@ import kotlin.collections.AbstractMap.SimpleEntry
  * have the same hash, each value in hashCodeMap is actually an array containing all entries whose
  * keys share the same hash.
  */
-internal class InternalHashCodeMap<K, V>(private val host: HashMap<K, V>) : InternalMap<K, V> {
+internal class InternalHashCodeMap<K, V>(override val equality: EqualityComparator) : InternalMap<K, V> {
 
     private var backingMap: dynamic = js("Object.create(null)")
     override var size: Int = 0
         private set
 
     override fun put(key: K, value: V): V? {
-        val hashCode = host.equality.getHashCode(key)
+        val hashCode = equality.getHashCode(key)
         val chain = getChainOrNull(hashCode)
         if (chain == null) {
             // This is a new chain, put it to the map.
@@ -62,11 +62,11 @@ internal class InternalHashCodeMap<K, V>(private val host: HashMap<K, V>) : Inte
     }
 
     override fun remove(key: K): V? {
-        val hashCode = host.equality.getHashCode(key)
+        val hashCode = equality.getHashCode(key)
         val chain = getChainOrNull(hashCode) ?: return null
         for (index in 0..chain.size-1) {
             val entry = chain[index]
-            if (host.equality.equals(key, entry.key)) {
+            if (equality.equals(key, entry.key)) {
                 if (chain.size == 1) {
                     chain.asDynamic().length = 0
                     // remove the whole array
@@ -94,10 +94,10 @@ internal class InternalHashCodeMap<K, V>(private val host: HashMap<K, V>) : Inte
     override fun get(key: K): V? = getEntry(key)?.value
 
     private fun getEntry(key: K): MutableEntry<K, V>? =
-            getChainOrNull(host.equality.getHashCode(key))?.findEntryInChain(key)
+            getChainOrNull(equality.getHashCode(key))?.findEntryInChain(key)
 
     private fun Array<MutableEntry<K, V>>.findEntryInChain(key: K): MutableEntry<K, V>? =
-            firstOrNull { entry -> host.equality.equals(entry.key, key) }
+            firstOrNull { entry -> equality.equals(entry.key, key) }
 
     override fun iterator(): MutableIterator<MutableEntry<K, V>> {
 

js/js.libraries/src/core/collections/InternalMap.kt

@@ -17,6 +17,7 @@
 package kotlin.collections
 
 internal interface InternalMap<K, V> : MutableIterable<MutableMap.MutableEntry<K, V>> {
+    val equality: EqualityComparator
     val size: Int
     operator fun contains(key: K): Boolean
     operator fun get(key: K): V?

js/js.libraries/src/core/collections/InternalStringMap.kt

@@ -24,7 +24,7 @@ import kotlin.collections.MutableMap.MutableEntry
 /**
  * A simple wrapper around JavaScript Map for key type is string.
  */
-internal class InternalStringMap<K, V>(private val host: HashMap<K, V>) : InternalMap<K, V> {
+internal class InternalStringMap<K, V>(override val equality: EqualityComparator) : InternalMap<K, V> {
 
     private var backingMap: dynamic = js("Object.create(null)")
     override var size: Int = 0

js/js.libraries/src/core/collections/LinkedHashMap.kt

@@ -162,35 +162,31 @@ open class LinkedHashMap<K, V> : HashMap<K, V>, Map<K, V> {
    * duplicate the key here to eliminate changes to HashMap and minimize the
    * code here, at the expense of additional space.
    */
-    private val map = HashMap<K, ChainEntry>()
+    private val map: HashMap<K, ChainEntry>
 
-    constructor() {
+    constructor() : super()  {
-        resetChainEntries()
+        map = HashMap<K, ChainEntry>()
     }
 
-    constructor(ignored: Int, alsoIgnored: Float = 0f) : super(ignored, alsoIgnored) {
+    internal constructor(backingMap: HashMap<K, Any>) : super() {
-        resetChainEntries()
+        map = backingMap as HashMap<K, ChainEntry>
     }
 
-//    constructor(ignored: Int, alsoIgnored: Float, accessOrder: Boolean) : super(ignored, alsoIgnored) {
+    constructor(initialCapacity: Int, loadFactor: Float = 0f) : super(initialCapacity, loadFactor) {
-//        this.accessOrder = accessOrder
+        map = HashMap<K, ChainEntry>()
-//        resetChainEntries()
+    }
-//    }
 
     constructor(original: Map<out K, V>) {
-        resetChainEntries()
+        map = HashMap<K, ChainEntry>()
         this.putAll(original)
     }
 
     override fun clear() {
         map.clear()
-        resetChainEntries()
-    }
-
-    private fun resetChainEntries() {
         head = null
     }
-//
+
+
 //    override fun clone(): Any {
 //        return LinkedHashMap(this)
 //    }
@@ -265,3 +261,8 @@ open class LinkedHashMap<K, V> : HashMap<K, V>, Map<K, V> {
 //        }
     }
 }
+
+
+public fun <V> linkedStringMapOf(vararg pairs: Pair<String, V>): LinkedHashMap<String, V> {
+    return LinkedHashMap<String, V>(stringMapOf<Any>()).apply { putAll(pairs) }
+}

js/js.libraries/src/core/collections/LinkedHashSet.kt

@@ -22,16 +22,22 @@ package kotlin.collections
 
 open class LinkedHashSet<E> : HashSet<E> {
 
+    internal constructor(map: LinkedHashMap<E, Any>) : super(map)
+
     constructor() : super(LinkedHashMap<E, Any>())
 
     constructor(c: Collection<E>) : super(LinkedHashMap<E, Any>()) {
         addAll(c)
     }
 
-    constructor(capacity: Int, loadFactor: Float = 0.0f) : super(LinkedHashMap<E, Any>(capacity, loadFactor))
+    constructor(initialCapacity: Int, loadFactor: Float = 0.0f) : super(LinkedHashMap<E, Any>(initialCapacity, loadFactor))
 
 //    public override fun clone(): Any {
 //        return LinkedHashSet(this)
 //    }
 
 }
+
+public fun linkedStringSetOf(vararg elements: String): LinkedHashSet<String> {
+    return LinkedHashSet(linkedStringMapOf<Any>()).apply { addAll(elements) }
+}

libraries/stdlib/test/js/MapJsTest.kt

@@ -55,7 +55,7 @@ class PrimitiveMapJsTest : MapJsTest() {
     }
 }
 
-class LinkedHashMapTest : MapJsTest() {
+class LinkedHashMapJsTest : MapJsTest() {
     @test override fun constructors() {
         LinkedHashMap<String, Int>()
         LinkedHashMap<String, Int>(3)
@@ -72,6 +72,19 @@ class LinkedHashMapTest : MapJsTest() {
     override fun emptyMutableMapWithNullableKeyValue(): MutableMap<String?, Int?> = LinkedHashMap()
 }
 
+class LinkedPrimitiveMapJsTest : MapJsTest() {
+    @test override fun constructors() {
+        val map = createTestMap()
+
+        assertEquals(KEYS.toNormalizedList(), map.keys.toNormalizedList())
+        assertEquals(VALUES.toNormalizedList(), map.values.toNormalizedList())
+    }
+
+    override fun <T : kotlin.Comparable<T>> Collection<T>.toNormalizedList(): List<T> = this.toList()
+    override fun emptyMutableMap(): MutableMap<String, Int> = linkedStringMapOf()
+    override fun emptyMutableMapWithNullableKeyValue(): MutableMap<String?, Int?> = LinkedHashMap()
+}
+
 abstract class MapJsTest {
     val KEYS = listOf("zero", "one", "two", "three")
     val VALUES = arrayOf(0, 1, 2, 3).toList()

libraries/stdlib/test/js/SetJsTest.kt

@@ -72,6 +72,18 @@ class LinkedHashSetJsTest : SetJsTest() {
     }
 }
 
+class LinkedPrimitiveSetJsTest : SetJsTest() {
+    override fun createEmptyMutableSet(): MutableSet<String> = linkedStringSetOf()
+    override fun createEmptyMutableSetWithNullableValues(): MutableSet<String?> = LinkedHashSet()
+    @Test
+    override fun constructors() {
+        val orderedData = data.toList()
+        val set = linkedStringSetOf(*orderedData.toTypedArray())
+
+        assertEquals(orderedData, set.toList())
+    }
+}
+
 abstract class SetJsTest {
     val data: Set<String> = createTestMutableSet()
     val empty: Set<String> = createEmptyMutableSet()

libraries/stdlib/test/js/StubsJVM.kt

@@ -19,4 +19,6 @@ package test.collections.js
 import java.util.*
 
 public fun <V> stringMapOf(vararg pairs: Pair<String, V>): HashMap<String, V> = hashMapOf<String, V>(*pairs)
+public fun <V> linkedStringMapOf(vararg pairs: Pair<String, V>): LinkedHashMap<String, V> = linkedMapOf(*pairs)
 public fun stringSetOf(vararg elements: String): HashSet<String> = hashSetOf(*elements)
+public fun linkedStringSetOf(vararg elements: String): LinkedHashSet<String> = linkedSetOf(*elements)