AK: Use a SinglyLinkedList<T> as HashTable's bucket chain storage.

We were using a DoublyLinkedList<T> simply because it supported remove().
This patch consolidates the SinglyLinkedList iterators and adds remove().

AK/HashTable.h

@@ -1,10 +1,10 @@
 #pragma once
 
-#include "Assertions.h"
-#include "DoublyLinkedList.h"
-#include "StdLibExtras.h"
-#include "Traits.h"
-#include "kstdio.h"
+#include <AK/Assertions.h>
+#include <AK/SinglyLinkedList.h>
+#include <AK/StdLibExtras.h>
+#include <AK/Traits.h>
+#include <AK/kstdio.h>
 
 namespace AK {
 
@@ -75,7 +75,7 @@ private:
 template<typename T, typename TraitsForT>
 class HashTable {
 private:
-    using Bucket = DoublyLinkedList<T>;
+    using Bucket = SinglyLinkedList<T>;
 
 public:
     HashTable() {}
@@ -136,12 +136,12 @@ public:
 
     void dump() const;
 
-    using Iterator = HashTableIterator<HashTable, T, typename DoublyLinkedList<T>::Iterator>;
+    using Iterator = HashTableIterator<HashTable, T, typename SinglyLinkedList<T>::Iterator>;
     friend Iterator;
     Iterator begin() { return Iterator(*this, is_empty()); }
     Iterator end() { return Iterator(*this, true); }
 
-    using ConstIterator = HashTableIterator<const HashTable, const T, typename DoublyLinkedList<T>::ConstIterator>;
+    using ConstIterator = HashTableIterator<const HashTable, const T, typename SinglyLinkedList<T>::ConstIterator>;
     friend ConstIterator;
     ConstIterator begin() const { return ConstIterator(*this, is_empty()); }
     ConstIterator end() const { return ConstIterator(*this, true); }

AK/SinglyLinkedList.h

@@ -4,6 +4,32 @@
 
 namespace AK {
 
+template<typename ListType, typename ElementType>
+class SinglyLinkedListIterator {
+public:
+    bool operator!=(const SinglyLinkedListIterator& other) const { return m_node != other.m_node; }
+    SinglyLinkedListIterator& operator++()
+    {
+        m_prev = m_node;
+        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 SinglyLinkedListIterator universal_end() { return SinglyLinkedListIterator(nullptr); }
+
+private:
+    friend ListType;
+    explicit SinglyLinkedListIterator(typename ListType::Node* node, typename ListType::Node* prev = nullptr)
+        : m_node(node)
+        , m_prev(prev)
+    {
+    }
+    typename ListType::Node* m_node { nullptr };
+    typename ListType::Node* m_prev { nullptr };
+};
+
 template<typename T>
 class SinglyLinkedList {
 private:
@@ -12,6 +38,10 @@ private:
             : value(move(v))
         {
         }
+        explicit Node(const T& v)
+            : value(v)
+        {
+        }
         T value;
         Node* next { nullptr };
     };
@@ -74,6 +104,18 @@ public:
         return value;
     }
 
+    void append(const T& value)
+    {
+        auto* node = new Node(value);
+        if (!m_head) {
+            m_head = node;
+            m_tail = node;
+            return;
+        }
+        m_tail->next = node;
+        m_tail = node;
+    }
+
     void append(T&& value)
     {
         auto* node = new Node(move(value));
@@ -95,75 +137,51 @@ public:
         return false;
     }
 
-    class Iterator {
-    public:
-        bool operator!=(const Iterator& other) { return m_node != other.m_node; }
-        Iterator& operator++()
-        {
-            m_node = m_node->next;
-            return *this;
-        }
-        T& operator*() { return m_node->value; }
-        bool is_end() const { return !m_node; }
-        static Iterator universal_end() { return Iterator(nullptr); }
-
-    private:
-        friend class SinglyLinkedList;
-        explicit Iterator(SinglyLinkedList::Node* node)
-            : m_node(node)
-        {
-        }
-        SinglyLinkedList::Node* m_node;
-    };
-
+    using Iterator = SinglyLinkedListIterator<SinglyLinkedList, T>;
+    friend Iterator;
     Iterator begin() { return Iterator(m_head); }
     Iterator end() { return Iterator::universal_end(); }
 
-    class ConstIterator {
-    public:
-        bool operator!=(const ConstIterator& other) { return m_node != other.m_node; }
-        ConstIterator& operator++()
-        {
-            m_node = m_node->next;
-            return *this;
-        }
-        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 SinglyLinkedList;
-        explicit ConstIterator(const SinglyLinkedList::Node* node)
-            : m_node(node)
-        {
-        }
-        const SinglyLinkedList::Node* m_node;
-    };
-
+    using ConstIterator = SinglyLinkedListIterator<const SinglyLinkedList, const T>;
+    friend ConstIterator;
     ConstIterator begin() const { return ConstIterator(m_head); }
     ConstIterator end() const { return ConstIterator::universal_end(); }
 
     ConstIterator find(const T& value) const
     {
+        Node* prev = nullptr;
         for (auto* node = m_head; node; node = node->next) {
             if (node->value == value)
-                return ConstIterator(node);
+                return ConstIterator(node, prev);
+            prev = node;
         }
         return end();
     }
 
     Iterator find(const T& value)
     {
+        Node* prev = nullptr;
         for (auto* node = m_head; node; node = node->next) {
             if (node->value == value)
-                return Iterator(node);
+                return Iterator(node, prev);
+            prev = node;
         }
         return end();
     }
 
-private:
-    friend class Iterator;
+    void remove(Iterator iterator)
+    {
+        ASSERT(!iterator.is_end());
+        if (m_head == iterator.m_node)
+            m_head = iterator.m_node->next;
+        if (m_tail == iterator.m_node)
+            m_tail = iterator.m_prev;
+        if (iterator.m_prev)
+            iterator.m_prev->next = iterator.m_node->next;
+        delete iterator.m_node;
+    }
 
+private:
     Node* head() { return m_head; }
     const Node* head() const { return m_head; }
 

AK/Tests/TestHashMap.cpp

@@ -23,6 +23,14 @@ int main()
         ++loop_counter;
     }
 
+    number_to_string.remove(1);
+    EXPECT_EQ(number_to_string.size(), 2);
+    EXPECT(number_to_string.find(1) == number_to_string.end());
+
+    number_to_string.remove(3);
+    EXPECT_EQ(number_to_string.size(), 1);
+    EXPECT(number_to_string.find(3) == number_to_string.end());
+
     EXPECT_EQ(loop_counter, 3);
     return 0;
 }