AK: Consolidate iterators for HashTable and DoublyLinkedList respectively.

Get rid of the ConstIterator classes for these containers and use templated
FooIterator<T, ...> and FooIterator<const T, ...> helpers.

This makes the HashTable class a lot easier to read.

AK/DoublyLinkedList.h

@@ -5,6 +5,30 @@
 
 namespace AK {
 
+template<typename ListType, typename ElementType>
+class DoublyLinkedListIterator {
+public:
+    bool operator!=(const DoublyLinkedListIterator& other) const { return m_node != other.m_node; }
+    bool operator==(const DoublyLinkedListIterator& other) const { return m_node == other.m_node; }
+    DoublyLinkedListIterator& operator++()
+    {
+        m_node = m_node->next;
+        return *this;
+    }
+    ElementType& operator*() { return m_node->value; }
+    ElementType* operator->() { return &m_node->value; }
+    bool is_end() const { return !m_node; }
+    static DoublyLinkedListIterator universal_end() { return DoublyLinkedListIterator(nullptr); }
+
+private:
+    friend ListType;
+    explicit DoublyLinkedListIterator(typename ListType::Node* node)
+        : m_node(node)
+    {
+    }
+    typename ListType::Node* m_node;
+};
+
 template<typename T>
 class DoublyLinkedList {
 private:
@@ -79,55 +103,13 @@ public:
         return false;
     }
 
-    class Iterator {
+    using Iterator = DoublyLinkedListIterator<DoublyLinkedList, T>;
-    public:
+    friend Iterator;
-        bool operator!=(const Iterator& other) const { return m_node != other.m_node; }
-        bool operator==(const Iterator& other) const { return m_node == other.m_node; }
-        Iterator& operator++()
-        {
-            m_node = m_node->next;
-            return *this;
-        }
-        T& operator*() { return m_node->value; }
-        T* operator->() { return &m_node->value; }
-        bool is_end() const { return !m_node; }
-        static Iterator universal_end() { return Iterator(nullptr); }
-
-    private:
-        friend class DoublyLinkedList;
-        explicit Iterator(DoublyLinkedList::Node* node)
-            : m_node(node)
-        {
-        }
-        DoublyLinkedList::Node* m_node;
-    };
-
     Iterator begin() { return Iterator(m_head); }
     Iterator end() { return Iterator::universal_end(); }
 
-    class ConstIterator {
+    using ConstIterator = DoublyLinkedListIterator<const DoublyLinkedList, const T>;
-    public:
+    friend ConstIterator;
-        bool operator!=(const ConstIterator& other) const { return m_node != other.m_node; }
-        bool operator==(const ConstIterator& other) const { return m_node == other.m_node; }
-        ConstIterator& operator++()
-        {
-            m_node = m_node->next;
-            return *this;
-        }
-        const T& operator*() const { return m_node->value; }
-        const T* operator->() const { return &m_node->value; }
-        bool is_end() const { return !m_node; }
-        static ConstIterator universal_end() { return ConstIterator(nullptr); }
-
-    private:
-        friend class DoublyLinkedList;
-        explicit ConstIterator(const DoublyLinkedList::Node* node)
-            : m_node(node)
-        {
-        }
-        const DoublyLinkedList::Node* m_node;
-    };
-
     ConstIterator begin() const { return ConstIterator(m_head); }
     ConstIterator end() const { return ConstIterator::universal_end(); }
 
@@ -171,8 +153,6 @@ public:
     }
 
 private:
-    friend class Iterator;
-
     void append_node(Node* node)
     {
         if (!m_head) {

AK/HashTable.h

@@ -6,19 +6,76 @@
 #include "Traits.h"
 #include "kstdio.h"
 
-//#define HASHTABLE_DEBUG
-
 namespace AK {
 
 template<typename T, typename = Traits<T>>
 class HashTable;
 
+template<typename HashTableType, typename ElementType, typename BucketIteratorType>
+class HashTableIterator {
+public:
+    bool operator!=(const HashTableIterator& other) const
+    {
+        if (m_is_end && other.m_is_end)
+            return false;
+        return &m_table != &other.m_table
+            || m_is_end != other.m_is_end
+            || m_bucket_index != other.m_bucket_index
+            || m_bucket_iterator != other.m_bucket_iterator;
+    }
+    bool operator==(const HashTableIterator& other) const { return !(*this != other); }
+    ElementType& operator*() { return *m_bucket_iterator; }
+    ElementType* operator->() { return m_bucket_iterator.operator->(); }
+    HashTableIterator& operator++()
+    {
+        skip_to_next();
+        return *this;
+    }
+
+    void skip_to_next()
+    {
+        while (!m_is_end) {
+            if (m_bucket_iterator.is_end()) {
+                ++m_bucket_index;
+                if (m_bucket_index >= m_table.capacity()) {
+                    m_is_end = true;
+                    return;
+                }
+                m_bucket_iterator = m_table.bucket(m_bucket_index).begin();
+            } else {
+                ++m_bucket_iterator;
+            }
+            if (!m_bucket_iterator.is_end())
+                return;
+        }
+    }
+
+private:
+    friend HashTableType;
+
+    explicit HashTableIterator(HashTableType& table, bool is_end, BucketIteratorType bucket_iterator = BucketIteratorType::universal_end(), int bucket_index = 0)
+        : m_table(table)
+        , m_bucket_index(bucket_index)
+        , m_is_end(is_end)
+        , m_bucket_iterator(bucket_iterator)
+    {
+        if (!is_end && !m_table.is_empty() && !(m_bucket_iterator != BucketIteratorType::universal_end())) {
+            m_bucket_iterator = m_table.bucket(0).begin();
+            if (m_bucket_iterator.is_end())
+                skip_to_next();
+        }
+    }
+
+    HashTableType& m_table;
+    int m_bucket_index { 0 };
+    bool m_is_end { false };
+    BucketIteratorType m_bucket_iterator;
+};
+
 template<typename T, typename TraitsForT>
 class HashTable {
 private:
-    struct Bucket {
+    using Bucket = DoublyLinkedList<T>;
-        DoublyLinkedList<T> chain;
-    };
 
 public:
     HashTable() {}
@@ -79,161 +136,13 @@ public:
 
     void dump() const;
 
-    class Iterator {
+    using Iterator = HashTableIterator<HashTable, T, typename DoublyLinkedList<T>::Iterator>;
-    public:
+    friend Iterator;
-        bool operator!=(const Iterator& other) const
-        {
-            if (m_is_end && other.m_is_end)
-                return false;
-            return &m_table != &other.m_table
-                || m_is_end != other.m_is_end
-                || m_bucket_index != other.m_bucket_index
-                || m_bucket_iterator != other.m_bucket_iterator;
-        }
-        bool operator==(const Iterator& other) const { return !(*this != other); }
-        T& operator*()
-        {
-#ifdef HASHTABLE_DEBUG
-            kprintf("retrieve { bucket_index: %u, is_end: %u }\n", m_bucket_index, m_is_end);
-#endif
-            return *m_bucket_iterator;
-        }
-        T* operator->() { return m_bucket_iterator.operator->(); }
-        Iterator& operator++()
-        {
-            skip_to_next();
-            return *this;
-        }
-
-        void skip_to_next()
-        {
-#ifdef HASHTABLE_DEBUG
-            unsigned pass = 0;
-#endif
-            while (!m_is_end) {
-#ifdef HASHTABLE_DEBUG
-                ++pass;
-                kprintf("skip_to_next pass %u, m_bucket_index=%u\n", pass, m_bucket_index);
-#endif
-                if (m_bucket_iterator.is_end()) {
-                    ++m_bucket_index;
-                    if (m_bucket_index >= m_table.capacity()) {
-                        m_is_end = true;
-                        return;
-                    }
-                    m_bucket_iterator = m_table.m_buckets[m_bucket_index].chain.begin();
-                } else {
-                    ++m_bucket_iterator;
-                }
-                if (!m_bucket_iterator.is_end())
-                    return;
-            }
-        }
-
-    private:
-        friend class HashTable;
-        explicit Iterator(HashTable& table, bool is_end, typename DoublyLinkedList<T>::Iterator bucket_iterator = DoublyLinkedList<T>::Iterator::universal_end(), int bucket_index = 0)
-            : m_table(table)
-            , m_bucket_index(bucket_index)
-            , m_is_end(is_end)
-            , m_bucket_iterator(bucket_iterator)
-        {
-            if (!is_end && !m_table.is_empty() && !(m_bucket_iterator != DoublyLinkedList<T>::Iterator::universal_end())) {
-#ifdef HASHTABLE_DEBUG
-                kprintf("bucket iterator init!\n");
-#endif
-                m_bucket_iterator = m_table.m_buckets[0].chain.begin();
-                if (m_bucket_iterator.is_end())
-                    skip_to_next();
-            }
-        }
-
-        HashTable& m_table;
-        int m_bucket_index { 0 };
-        bool m_is_end { false };
-        typename DoublyLinkedList<T>::Iterator m_bucket_iterator;
-    };
-
     Iterator begin() { return Iterator(*this, is_empty()); }
     Iterator end() { return Iterator(*this, true); }
 
-    class ConstIterator {
+    using ConstIterator = HashTableIterator<const HashTable, const T, typename DoublyLinkedList<T>::ConstIterator>;
-    public:
+    friend ConstIterator;
-        bool operator!=(const ConstIterator& other) const
-        {
-            if (m_is_end && other.m_is_end)
-                return false;
-            return &m_table != &other.m_table
-                || m_is_end != other.m_is_end
-                || m_bucket_index != other.m_bucket_index
-                || m_bucket_iterator != other.m_bucket_iterator;
-        }
-        bool operator==(const ConstIterator& other) const { return !(*this != other); }
-        const T& operator*() const
-        {
-#ifdef HASHTABLE_DEBUG
-            kprintf("retrieve { bucket_index: %u, is_end: %u }\n", m_bucket_index, m_is_end);
-#endif
-            return *m_bucket_iterator;
-        }
-        const T* operator->() const { return m_bucket_iterator.operator->(); }
-        ConstIterator& operator++()
-        {
-            skip_to_next();
-            return *this;
-        }
-
-        void skip_to_next()
-        {
-#ifdef HASHTABLE_DEBUG
-            unsigned pass = 0;
-#endif
-            while (!m_is_end) {
-#ifdef HASHTABLE_DEBUG
-                ++pass;
-                kprintf("skip_to_next pass %u, m_bucket_index=%u\n", pass, m_bucket_index);
-#endif
-                if (m_bucket_iterator.is_end()) {
-                    ++m_bucket_index;
-                    if (m_bucket_index >= m_table.capacity()) {
-                        m_is_end = true;
-                        return;
-                    }
-                    const DoublyLinkedList<T>& chain = m_table.m_buckets[m_bucket_index].chain;
-                    m_bucket_iterator = chain.begin();
-                } else {
-                    ++m_bucket_iterator;
-                }
-                if (!m_bucket_iterator.is_end())
-                    return;
-            }
-        }
-
-    private:
-        friend class HashTable;
-        ConstIterator(const HashTable& table, bool is_end, typename DoublyLinkedList<T>::ConstIterator bucket_iterator = DoublyLinkedList<T>::ConstIterator::universal_end(), int bucket_index = 0)
-            : m_table(table)
-            , m_bucket_index(bucket_index)
-            , m_is_end(is_end)
-            , m_bucket_iterator(bucket_iterator)
-        {
-            if (!is_end && !m_table.is_empty() && !(m_bucket_iterator != DoublyLinkedList<T>::ConstIterator::universal_end())) {
-#ifdef HASHTABLE_DEBUG
-                kprintf("const bucket iterator init!\n");
-#endif
-                const DoublyLinkedList<T>& chain = m_table.m_buckets[0].chain;
-                m_bucket_iterator = chain.begin();
-                if (m_bucket_iterator.is_end())
-                    skip_to_next();
-            }
-        }
-
-        const HashTable& m_table;
-        int m_bucket_index { 0 };
-        bool m_is_end { false };
-        typename DoublyLinkedList<T>::ConstIterator m_bucket_iterator;
-    };
-
     ConstIterator begin() const { return ConstIterator(*this, is_empty()); }
     ConstIterator end() const { return ConstIterator(*this, true); }
 
@@ -256,6 +165,9 @@ private:
     void insert(const T&);
     void insert(T&&);
 
+    Bucket& bucket(int index) { return m_buckets[index]; }
+    const Bucket& bucket(int index) const { return m_buckets[index]; }
+
     Bucket* m_buckets { nullptr };
 
     int m_size { 0 };
@@ -268,7 +180,7 @@ void HashTable<T, TraitsForT>::set(T&& value)
     if (!m_capacity)
         rehash(1);
     auto& bucket = lookup(value);
-    for (auto& e : bucket.chain) {
+    for (auto& e : bucket) {
         if (e == value) {
             e = move(value);
             return;
@@ -278,7 +190,7 @@ void HashTable<T, TraitsForT>::set(T&& value)
         rehash(size() + 1);
         insert(move(value));
     } else {
-        bucket.chain.append(move(value));
+        bucket.append(move(value));
     }
     m_size++;
 }
@@ -289,7 +201,7 @@ void HashTable<T, TraitsForT>::set(const T& value)
     if (!m_capacity)
         rehash(1);
     auto& bucket = lookup(value);
-    for (auto& e : bucket.chain) {
+    for (auto& e : bucket) {
         if (e == value) {
             e = move(value);
             return;
@@ -299,7 +211,7 @@ void HashTable<T, TraitsForT>::set(const T& value)
         rehash(size() + 1);
         insert(value);
     } else {
-        bucket.chain.append(value);
+        bucket.append(value);
     }
     m_size++;
 }
@@ -308,20 +220,14 @@ template<typename T, typename TraitsForT>
 void HashTable<T, TraitsForT>::rehash(int new_capacity)
 {
     new_capacity *= 2;
-#ifdef HASHTABLE_DEBUG
-    kprintf("rehash to %u buckets\n", new_capacity);
-#endif
     auto* new_buckets = new Bucket[new_capacity];
     auto* old_buckets = m_buckets;
     int old_capacity = m_capacity;
     m_buckets = new_buckets;
     m_capacity = new_capacity;
 
-#ifdef HASHTABLE_DEBUG
-    kprintf("reinsert %u buckets\n", old_capacity);
-#endif
     for (int i = 0; i < old_capacity; ++i) {
-        for (auto& value : old_buckets[i].chain) {
+        for (auto& value : old_buckets[i]) {
             insert(move(value));
         }
     }
@@ -344,14 +250,14 @@ template<typename T, typename TraitsForT>
 void HashTable<T, TraitsForT>::insert(T&& value)
 {
     auto& bucket = lookup(value);
-    bucket.chain.append(move(value));
+    bucket.append(move(value));
 }
 
 template<typename T, typename TraitsForT>
 void HashTable<T, TraitsForT>::insert(const T& value)
 {
     auto& bucket = lookup(value);
-    bucket.chain.append(value);
+    bucket.append(value);
 }
 
 template<typename T, typename TraitsForT>
@@ -360,7 +266,7 @@ bool HashTable<T, TraitsForT>::contains(const T& value) const
     if (is_empty())
         return false;
     auto& bucket = lookup(value);
-    for (auto& e : bucket.chain) {
+    for (auto& e : bucket) {
         if (e == value)
             return true;
     }
@@ -374,8 +280,8 @@ auto HashTable<T, TraitsForT>::find(const T& value) -> Iterator
         return end();
     int bucket_index;
     auto& bucket = lookup(value, &bucket_index);
-    auto bucket_iterator = bucket.chain.find(value);
+    auto bucket_iterator = bucket.find(value);
-    if (bucket_iterator != bucket.chain.end())
+    if (bucket_iterator != bucket.end())
         return Iterator(*this, false, bucket_iterator, bucket_index);
     return end();
 }
@@ -386,9 +292,9 @@ auto HashTable<T, TraitsForT>::find(const T& value) const -> ConstIterator
     if (is_empty())
         return end();
     int bucket_index;
-    auto& bucket = lookup(value, &bucket_index);
+    const auto& bucket = lookup(value, &bucket_index);
-    auto bucket_iterator = bucket.chain.find(value);
+    auto bucket_iterator = bucket.find(value);
-    if (bucket_iterator != bucket.chain.end())
+    if (bucket_iterator != bucket.end())
         return ConstIterator(*this, false, bucket_iterator, bucket_index);
     return end();
 }
@@ -397,33 +303,23 @@ template<typename T, typename TraitsForT>
 void HashTable<T, TraitsForT>::remove(Iterator it)
 {
     ASSERT(!is_empty());
-    m_buckets[it.m_bucket_index].chain.remove(it.m_bucket_iterator);
+    m_buckets[it.m_bucket_index].remove(it.m_bucket_iterator);
     --m_size;
 }
 
 template<typename T, typename TraitsForT>
-typename HashTable<T, TraitsForT>::Bucket& HashTable<T, TraitsForT>::lookup(const T& value, int* bucket_index)
+auto HashTable<T, TraitsForT>::lookup(const T& value, int* bucket_index) -> Bucket&
 {
     unsigned hash = TraitsForT::hash(value);
-#ifdef HASHTABLE_DEBUG
-    kprintf("hash for ");
-    TraitsForT::dump(value);
-    kprintf(" is %u\n", hash);
-#endif
     if (bucket_index)
         *bucket_index = hash % m_capacity;
     return m_buckets[hash % m_capacity];
 }
 
 template<typename T, typename TraitsForT>
-const typename HashTable<T, TraitsForT>::Bucket& HashTable<T, TraitsForT>::lookup(const T& value, int* bucket_index) const
+auto HashTable<T, TraitsForT>::lookup(const T& value, int* bucket_index) const -> const Bucket&
 {
     unsigned hash = TraitsForT::hash(value);
-#ifdef HASHTABLE_DEBUG
-    kprintf("hash for ");
-    TraitsForT::dump(value);
-    kprintf(" is %u\n", hash);
-#endif
     if (bucket_index)
         *bucket_index = hash % m_capacity;
     return m_buckets[hash % m_capacity];
@@ -436,7 +332,7 @@ void HashTable<T, TraitsForT>::dump() const
     for (int i = 0; i < m_capacity; ++i) {
         auto& bucket = m_buckets[i];
         kprintf("Bucket %u\n", i);
-        for (auto& e : bucket.chain) {
+        for (auto& e : bucket) {
             kprintf("  > ");
             TraitsForT::dump(e);
             kprintf("\n");

AK/Tests/.gitignore

@@ -1,5 +1,6 @@
 TestString
 TestQueue
 TestVector
+TestHashMap
 *.d
 *.o

AK/Tests/Makefile

@@ -1,4 +1,6 @@
-all: TestString TestQueue TestVector
+PROGRAMS = TestString TestQueue TestVector TestHashMap
+
+all: $(PROGRAMS)
 
 CXXFLAGS = -std=c++17 -Wall -Wextra
 
@@ -11,5 +13,8 @@ TestQueue: TestQueue.cpp ../String.cpp ../StringImpl.cpp ../StringBuilder.cpp ..
 TestVector: TestVector.cpp ../String.cpp ../StringImpl.cpp ../StringBuilder.cpp ../StringView.cpp TestHelpers.h
 	$(CXX) $(CXXFLAGS) -I../ -I../../ -o $@ TestVector.cpp ../String.cpp ../StringImpl.cpp ../StringBuilder.cpp ../StringView.cpp
 
+TestHashMap: TestHashMap.cpp ../String.cpp ../StringImpl.cpp ../StringBuilder.cpp ../StringView.cpp TestHelpers.h
+	$(CXX) $(CXXFLAGS) -I../ -I../../ -o $@ TestHashMap.cpp ../String.cpp ../StringImpl.cpp ../StringBuilder.cpp ../StringView.cpp
+
 clean:
-	rm -f TestString TestQueue
+	rm -f $(PROGRAMS)

AK/Tests/TestHashMap.cpp

@@ -0,0 +1,28 @@
+#include "TestHelpers.h"
+#include <AK/AKString.h>
+#include <AK/HashMap.h>
+
+typedef HashMap<int, int> IntIntMap;
+
+int main()
+{
+    EXPECT(IntIntMap().is_empty());
+    EXPECT(IntIntMap().size() == 0);
+
+    HashMap<int, String> number_to_string;
+    number_to_string.set(1, "One");
+    number_to_string.set(2, "Two");
+    number_to_string.set(3, "Three");
+    
+    EXPECT_EQ(number_to_string.is_empty(), false);
+    EXPECT_EQ(number_to_string.size(), 3);
+
+    int loop_counter = 0;
+    for (auto& it : number_to_string) {
+        EXPECT(!it.value.is_null());
+        ++loop_counter;
+    }
+
+    EXPECT_EQ(loop_counter, 3);
+    return 0;
+}