From 623bdd8b6a818504db3c9df02d99b92ac7acb92d Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sun, 6 Mar 2022 19:11:17 +0100
Subject: [PATCH] AK: Simplify HashTable::remove_all_matching()

Just walk the table from start to finish, deleting buckets as we go.
This removes the need for remove() to return an iterator, which is
preventing me from implementing hash table auto-shrinking.
---
 AK/HashMap.h   | 13 +++---------
 AK/HashTable.h | 55 +++++++++++++++++++++++++++++---------------------
 2 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/AK/HashMap.h b/AK/HashMap.h
index 2fde4b9b7b..3ba4f424bf 100644
--- a/AK/HashMap.h
+++ b/AK/HashMap.h
@@ -79,16 +79,9 @@ public:
     template<typename TUnaryPredicate>
     bool remove_all_matching(TUnaryPredicate predicate)
     {
-        bool something_was_removed = false;
-        for (auto it = begin(); it != end();) {
-            if (predicate(it->key, it->value)) {
-                it = remove(it);
-                something_was_removed = true;
-            } else {
-                ++it;
-            }
-        }
-        return something_was_removed;
+        return m_table.template remove_all_matching([&](auto& entry) {
+            return predicate(entry.key, entry.value);
+        });
     }
 
     using HashTableType = HashTable<Entry, EntryTraits, IsOrdered>;
diff --git a/AK/HashTable.h b/AK/HashTable.h
index 6b797b4fcb..784c851a91 100644
--- a/AK/HashTable.h
+++ b/AK/HashTable.h
@@ -402,40 +402,30 @@ public:
         auto next_iterator = iterator;
         ++next_iterator;
 
-        bucket.slot()->~T();
-        bucket.used = false;
-        bucket.deleted = true;
+        delete_bucket(bucket);
         --m_size;
         ++m_deleted_count;
 
-        if constexpr (IsOrdered) {
-            if (bucket.previous)
-                bucket.previous->next = bucket.next;
-            else
-                m_collection_data.head = bucket.next;
-
-            if (bucket.next)
-                bucket.next->previous = bucket.previous;
-            else
-                m_collection_data.tail = bucket.previous;
-        }
-
         return next_iterator;
     }
 
     template<typename TUnaryPredicate>
     bool remove_all_matching(TUnaryPredicate predicate)
     {
-        bool something_was_removed = false;
-        for (auto it = begin(); it != end();) {
-            if (predicate(*it)) {
-                it = remove(it);
-                something_was_removed = true;
-            } else {
-                ++it;
+        size_t removed_count = 0;
+        for (size_t i = 0; i < m_capacity; ++i) {
+            auto& bucket = m_buckets[i];
+            if (bucket.used && predicate(*bucket.slot())) {
+                delete_bucket(bucket);
+                ++removed_count;
             }
         }
-        return something_was_removed;
+        if (removed_count) {
+            m_deleted_count += removed_count;
+            m_size -= removed_count;
+            return true;
+        }
+        return false;
     }
 
 private:
@@ -558,6 +548,25 @@ private:
     [[nodiscard]] size_t used_bucket_count() const { return m_size + m_deleted_count; }
     [[nodiscard]] bool should_grow() const { return ((used_bucket_count() + 1) * 100) >= (m_capacity * load_factor_in_percent); }
 
+    void delete_bucket(auto& bucket)
+    {
+        bucket.slot()->~T();
+        bucket.used = false;
+        bucket.deleted = true;
+
+        if constexpr (IsOrdered) {
+            if (bucket.previous)
+                bucket.previous->next = bucket.next;
+            else
+                m_collection_data.head = bucket.next;
+
+            if (bucket.next)
+                bucket.next->previous = bucket.previous;
+            else
+                m_collection_data.tail = bucket.previous;
+        }
+    }
+
     BucketType* m_buckets { nullptr };
 
     [[no_unique_address]] CollectionDataType m_collection_data;