AK: Combine SinglyLinkedList and SinglyLinkedListWithCount

Using policy based design `SinglyLinkedList` and
`SinglyLinkedListWithCount` can be combined into one class which takes
a policy to determine how to keep track of the size of the list. The
default policy is to use list iteration to count the items in the list
each time. The `WithCount` form is a different policy which tracks the
size, but comes with the overhead of storing the count and
incrementing/decrementing on each modification.

This model is extensible to have other forms of counting by
implementing only a new policy instead of implementing a totally new
type.

AK/Forward.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <AK/DefaultDelete.h>
+#include <AK/SinglyLinkedListSizePolicy.h>
 #include <AK/Types.h>
 
 namespace AK {
@@ -64,7 +65,7 @@ using Bytes = Span<u8>;
 template<typename T, AK::MemoryOrder DefaultMemoryOrder>
 class Atomic;
 
-template<typename T>
+template<typename T, typename TSizeCalculationPolicy = DefaultSizeCalculationPolicy>
 class SinglyLinkedList;
 
 template<typename T>

AK/SinglyLinkedList.h

@@ -63,7 +63,7 @@ private:
     bool m_removed { false };
 };
 
-template<typename T>
+template<typename T, typename TSizeCalculationPolicy>
 class SinglyLinkedList {
 private:
     struct Node {
@@ -96,12 +96,9 @@ public:
 
     bool is_empty() const { return !head(); }
 
-    inline size_t size_slow() const
+    inline size_t size() const
     {
-        size_t size = 0;
-        for (auto* node = m_head; node; node = node->next)
-            ++size;
-        return size;
+        return m_size_policy.size(m_head);
     }
 
     void clear()
@@ -113,6 +110,7 @@ public:
         }
         m_head = nullptr;
         m_tail = nullptr;
+        m_size_policy.reset();
     }
 
     T& first()
@@ -144,6 +142,7 @@ public:
         if (m_tail == m_head)
             m_tail = nullptr;
         m_head = m_head->next;
+        m_size_policy.decrease_size(value);
         delete prev_head;
         return value;
     }
@@ -154,6 +153,7 @@ public:
         auto* node = new (nothrow) Node(forward<U>(value));
         if (!node)
             return Error::from_errno(ENOMEM);
+        m_size_policy.increase_size(value);
         if (!m_head) {
             m_head = node;
             m_tail = node;
@@ -170,6 +170,7 @@ public:
         auto* node = new (nothrow) Node(forward<U>(value));
         if (!node)
             return Error::from_errno(ENOMEM);
+        m_size_policy.increase_size(value);
         if (!m_head) {
             m_head = node;
             m_tail = node;
@@ -237,6 +238,7 @@ public:
         auto* node = new (nothrow) Node(forward<U>(value));
         if (!node)
             return Error::from_errno(ENOMEM);
+        m_size_policy.increase_size(value);
         node->next = iterator.m_node;
         if (m_head == iterator.m_node)
             m_head = node;
@@ -254,6 +256,7 @@ public:
         auto* node = new (nothrow) Node(forward<U>(value));
         if (!node)
             return Error::from_errno(ENOMEM);
+        m_size_policy.increase_size(value);
         node->next = iterator.m_node->next;
 
         iterator.m_node->next = node;
@@ -286,6 +289,7 @@ public:
             m_tail = iterator.m_prev;
         if (iterator.m_prev)
             iterator.m_prev->next = iterator.m_node->next;
+        m_size_policy.decrease_size(iterator.m_node->value);
         delete iterator.m_node;
     }
 
@@ -298,8 +302,8 @@ private:
 
     Node* m_head { nullptr };
     Node* m_tail { nullptr };
+    TSizeCalculationPolicy m_size_policy {};
 };
-
 }
 
 #if USING_AK_GLOBALLY

AK/SinglyLinkedListSizePolicy.h

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2022, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/Types.h>
+
+namespace AK {
+
+struct DefaultSizeCalculationPolicy {
+    constexpr void increase_size(auto const&) { }
+
+    constexpr void decrease_size(auto const&) { }
+
+    constexpr void reset() { }
+
+    constexpr size_t size(auto const* head) const
+    {
+        size_t size = 0;
+        for (auto* node = head; node; node = node->next)
+            ++size;
+        return size;
+    }
+};
+
+struct CountingSizeCalculationPolicy {
+    constexpr void increase_size(auto const&) { ++m_size; }
+
+    constexpr void decrease_size(auto const&) { --m_size; }
+
+    constexpr void reset() { m_size = 0; }
+
+    constexpr size_t size(auto const*) const { return m_size; }
+
+private:
+    size_t m_size { 0 };
+};
+
+}
+
+#ifdef USING_AK_GLOBALLY
+using AK::CountingSizeCalculationPolicy;
+using AK::DefaultSizeCalculationPolicy;
+#endif

AK/SinglyLinkedListWithCount.h

@@ -1,144 +0,0 @@
-/*
- * Copyright (c) 2021, Brian Gianforcaro <bgianf@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/SinglyLinkedList.h>
-
-namespace AK {
-
-template<typename T>
-class SinglyLinkedListWithCount : private SinglyLinkedList<T> {
-
-public:
-    SinglyLinkedListWithCount() = default;
-    ~SinglyLinkedListWithCount() = default;
-
-    using List = SinglyLinkedList<T>;
-
-    using List::is_empty;
-    using List::size_slow;
-
-    inline size_t size() const
-    {
-        return m_count;
-    }
-
-    void clear()
-    {
-        List::clear();
-        m_count = 0;
-    }
-
-    T& first()
-    {
-        return List::first();
-    }
-
-    T const& first() const
-    {
-        return List::first();
-    }
-
-    T& last()
-    {
-        return List::last();
-    }
-
-    T const& last() const
-    {
-        return List::last();
-    }
-
-    T take_first()
-    {
-        m_count--;
-        return List::take_first();
-    }
-
-    template<typename U = T>
-    ErrorOr<void> try_append(U&& value)
-    {
-        auto result = List::try_append(forward<T>(value));
-        if (!result.is_error())
-            m_count++;
-        return result;
-    }
-
-#ifndef KERNEL
-    template<typename U = T>
-    void append(U&& value)
-    {
-        MUST(try_append(forward<T>(value)));
-    }
-#endif
-
-    bool contains_slow(T const& value) const
-    {
-        return List::contains_slow(value);
-    }
-
-    using Iterator = typename List::Iterator;
-    friend Iterator;
-    Iterator begin() { return List::begin(); }
-    Iterator end() { return List::end(); }
-
-    using ConstIterator = typename List::ConstIterator;
-    friend ConstIterator;
-    ConstIterator begin() const { return List::begin(); }
-    ConstIterator end() const { return List::end(); }
-
-    template<typename TUnaryPredicate>
-    ConstIterator find(TUnaryPredicate&& pred) const
-    {
-        return List::find_if(forward<TUnaryPredicate>(pred));
-    }
-
-    template<typename TUnaryPredicate>
-    Iterator find(TUnaryPredicate&& pred)
-    {
-        return List::find_if(forward<TUnaryPredicate>(pred));
-    }
-
-    ConstIterator find(T const& value) const
-    {
-        return List::find(value);
-    }
-
-    Iterator find(T const& value)
-    {
-        return List::find(value);
-    }
-
-    void remove(Iterator iterator)
-    {
-        m_count--;
-        return List::remove(iterator);
-    }
-
-    template<typename U = T>
-    void insert_before(Iterator iterator, U&& value)
-    {
-        m_count++;
-        List::insert_before(iterator, forward<T>(value));
-    }
-
-    template<typename U = T>
-    void insert_after(Iterator iterator, U&& value)
-    {
-        m_count++;
-        List::insert_after(iterator, forward<T>(value));
-    }
-
-private:
-    size_t m_count { 0 };
-};
-
-}
-
-#if USING_AK_GLOBALLY
-using AK::SinglyLinkedListWithCount;
-#endif

Kernel/Net/IPv4Socket.h

@@ -7,7 +7,7 @@
 #pragma once
 
 #include <AK/HashMap.h>
-#include <AK/SinglyLinkedListWithCount.h>
+#include <AK/SinglyLinkedList.h>
 #include <Kernel/DoubleBuffer.h>
 #include <Kernel/KBuffer.h>
 #include <Kernel/Locking/MutexProtected.h>
@@ -116,7 +116,7 @@ private:
         OwnPtr<KBuffer> data;
     };
 
-    SinglyLinkedListWithCount<ReceivedPacket> m_receive_queue;
+    SinglyLinkedList<ReceivedPacket, CountingSizeCalculationPolicy> m_receive_queue;
 
     OwnPtr<DoubleBuffer> m_receive_buffer;
 

Tests/AK/TestSinglyLinkedList.cpp

@@ -63,10 +63,129 @@ TEST_CASE(should_find_const_with_predicate)
 TEST_CASE(removal_during_iteration)
 {
     auto list = make_list();
-    auto size = list.size_slow();
+    auto size = list.size();
 
     for (auto it = list.begin(); it != list.end(); ++it, --size) {
-        VERIFY(list.size_slow() == size);
+        VERIFY(list.size() == size);
         it.remove(list);
     }
 }
+
+static size_t calls_to_increase { 0 };
+static size_t calls_to_decrease { 0 };
+static size_t calls_to_reset { 0 };
+static size_t calls_to_get_size { 0 };
+
+static void setup()
+{
+    calls_to_increase = 0;
+    calls_to_decrease = 0;
+    calls_to_reset = 0;
+    calls_to_get_size = 0;
+}
+
+struct TestSizeCalculationPolicy {
+    void increase_size(auto const&) { ++calls_to_increase; }
+
+    void decrease_size(auto const&) { ++calls_to_decrease; }
+
+    void reset() { ++calls_to_reset; }
+
+    size_t size(auto const*) const
+    {
+        ++calls_to_get_size;
+        return 42;
+    }
+};
+
+TEST_CASE(should_increase_size_when_appending)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    list.append(0);
+    EXPECT_EQ(1u, calls_to_increase);
+}
+
+TEST_CASE(should_decrease_size_when_removing)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    list.append(0);
+    auto begin = list.begin();
+    list.remove(begin);
+    EXPECT_EQ(1u, calls_to_decrease);
+}
+
+TEST_CASE(should_reset_size_when_clearing)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    list.append(0);
+    list.clear();
+    EXPECT_EQ(1u, calls_to_reset);
+}
+
+TEST_CASE(should_get_size_from_policy)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    EXPECT_EQ(42u, list.size());
+    EXPECT_EQ(1u, calls_to_get_size);
+}
+
+TEST_CASE(should_decrease_size_when_taking_first)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    list.append(0);
+    list.take_first();
+    EXPECT_EQ(1u, calls_to_decrease);
+}
+
+TEST_CASE(should_increase_size_when_try_appending)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    MUST(list.try_append(0));
+    EXPECT_EQ(1u, calls_to_increase);
+}
+
+TEST_CASE(should_increase_size_when_try_prepending)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    MUST(list.try_prepend(0));
+    EXPECT_EQ(1u, calls_to_increase);
+}
+
+TEST_CASE(should_increase_size_when_try_inserting_before)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    MUST(list.try_insert_before(list.begin(), 42));
+    EXPECT_EQ(1u, calls_to_increase);
+}
+
+TEST_CASE(should_increase_size_when_try_inserting_after)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    MUST(list.try_insert_after(list.begin(), 42));
+    EXPECT_EQ(1u, calls_to_increase);
+}
+
+TEST_CASE(should_increase_size_when_inserting_before)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    list.insert_before(list.begin(), 42);
+    EXPECT_EQ(1u, calls_to_increase);
+}
+
+TEST_CASE(should_increase_size_when_inserting_after)
+{
+    setup();
+    SinglyLinkedList<int, TestSizeCalculationPolicy> list {};
+    list.insert_after(list.begin(), 42);
+    EXPECT_EQ(1u, calls_to_increase);
+}