Browse Source

DoublyLinkedList: Implement `find` in terms of `AK::find`

Problem:
- The implementation of `find` is coupled to the implementation of
  `DoublyLinkedList`.
- `append` and `prepend` are implemented multiple times so that
  r-value references can be moved from into the new node. This is
  probably not called very often because a pr-value or x-value needs
  to be used here.

Solution:
- Decouple the implementation of `find` from the class by using a
  generic `find` algorithm.
- Make `append` and `prepend` be function templates so that they can
  have binding references which can be forwarded.
Lenny Maiorani 4 years ago
parent
commit
853cb8af5c
4 changed files with 114 additions and 84 deletions
  1. 47 83
      AK/DoublyLinkedList.h
  2. 1 0
      AK/Tests/CMakeLists.txt
  3. 65 0
      AK/Tests/TestDoublyLinkedList.cpp
  4. 1 1
      Kernel/Devices/Device.h

+ 47 - 83
AK/DoublyLinkedList.h

@@ -27,6 +27,7 @@
 #pragma once
 #pragma once
 
 
 #include <AK/Assertions.h>
 #include <AK/Assertions.h>
+#include <AK/Find.h>
 #include <AK/StdLibExtras.h>
 #include <AK/StdLibExtras.h>
 #include <AK/Traits.h>
 #include <AK/Traits.h>
 
 
@@ -60,13 +61,11 @@ template<typename T>
 class DoublyLinkedList {
 class DoublyLinkedList {
 private:
 private:
     struct Node {
     struct Node {
-        explicit Node(const T& v)
-            : value(v)
-        {
-        }
-        explicit Node(T&& v)
-            : value(move(v))
+        template<typename U>
+        explicit Node(U&& v)
+            : value(forward<U>(v))
         {
         {
+            static_assert(IsSame<T, U>::value);
         }
         }
         T value;
         T value;
         Node* next { nullptr };
         Node* next { nullptr };
@@ -77,7 +76,7 @@ public:
     DoublyLinkedList() { }
     DoublyLinkedList() { }
     ~DoublyLinkedList() { clear(); }
     ~DoublyLinkedList() { clear(); }
 
 
-    bool is_empty() const { return !head(); }
+    bool is_empty() const { return !m_head; }
 
 
     void clear()
     void clear()
     {
     {
@@ -92,48 +91,64 @@ public:
 
 
     T& first()
     T& first()
     {
     {
-        ASSERT(head());
-        return head()->value;
+        ASSERT(m_head);
+        return m_head->value;
     }
     }
     const T& first() const
     const T& first() const
     {
     {
-        ASSERT(head());
-        return head()->value;
+        ASSERT(m_head);
+        return m_head->value;
     }
     }
     T& last()
     T& last()
     {
     {
-        ASSERT(head());
-        return tail()->value;
+        ASSERT(m_head);
+        return m_tail->value;
     }
     }
     const T& last() const
     const T& last() const
     {
     {
-        ASSERT(head());
-        return tail()->value;
-    }
-
-    void append(T&& value)
-    {
-        append_node(new Node(move(value)));
-    }
-
-    void append(const T& value)
-    {
-        append_node(new Node(value));
+        ASSERT(m_head);
+        return m_tail->value;
     }
     }
 
 
-    void prepend(T&& value)
+    template<typename U>
+    void append(U&& value)
     {
     {
-        prepend_node(new Node(move(value)));
+        static_assert(IsSame<T, U>::value);
+        auto* node = new Node(forward<U>(value));
+        if (!m_head) {
+            ASSERT(!m_tail);
+            m_head = node;
+            m_tail = node;
+            return;
+        }
+        ASSERT(m_tail);
+        ASSERT(!node->next);
+        m_tail->next = node;
+        node->prev = m_tail;
+        m_tail = node;
     }
     }
 
 
-    void prepend(const T& value)
+    template<typename U>
+    void prepend(U&& value)
     {
     {
-        prepend_node(new Node(value));
+        static_assert(IsSame<T, U>::value);
+        auto* node = new Node(forward<U>(value));
+        if (!m_head) {
+            ASSERT(!m_tail);
+            m_head = node;
+            m_tail = node;
+            return;
+        }
+        ASSERT(m_tail);
+        ASSERT(!node->prev);
+        m_head->prev = node;
+        node->next = m_head;
+        m_head = node;
     }
     }
 
 
     bool contains_slow(const T& value) const
     bool contains_slow(const T& value) const
     {
     {
-        return find_node(value) != nullptr;
+        return find(value) != end();
     }
     }
 
 
     using Iterator = DoublyLinkedListIterator<DoublyLinkedList, T>;
     using Iterator = DoublyLinkedListIterator<DoublyLinkedList, T>;
@@ -148,18 +163,12 @@ public:
 
 
     ConstIterator find(const T& value) const
     ConstIterator find(const T& value) const
     {
     {
-        Node* node = find_node(value);
-        if (node)
-            return ConstIterator(node);
-        return end();
+        return AK::find(begin(), end(), value);
     }
     }
 
 
     Iterator find(const T& value)
     Iterator find(const T& value)
     {
     {
-        Node* node = find_node(value);
-        if (node)
-            return Iterator(node);
-        return end();
+        return AK::find(begin(), end(), value);
     }
     }
 
 
     void remove(Iterator it)
     void remove(Iterator it)
@@ -184,51 +193,6 @@ public:
     }
     }
 
 
 private:
 private:
-    void append_node(Node* node)
-    {
-        if (!m_head) {
-            ASSERT(!m_tail);
-            m_head = node;
-            m_tail = node;
-            return;
-        }
-        ASSERT(m_tail);
-        ASSERT(!node->next);
-        m_tail->next = node;
-        node->prev = m_tail;
-        m_tail = node;
-    }
-
-    void prepend_node(Node* node)
-    {
-        if (!m_head) {
-            ASSERT(!m_tail);
-            m_head = node;
-            m_tail = node;
-            return;
-        }
-        ASSERT(m_tail);
-        ASSERT(!node->prev);
-        m_head->prev = node;
-        node->next = m_head;
-        m_head = node;
-    }
-
-    Node* find_node(const T& value) const
-    {
-        for (auto* node = m_head; node; node = node->next) {
-            if (Traits<T>::equals(node->value, value))
-                return node;
-        }
-        return nullptr;
-    }
-
-    Node* head() { return m_head; }
-    const Node* head() const { return m_head; }
-
-    Node* tail() { return m_tail; }
-    const Node* tail() const { return m_tail; }
-
     Node* m_head { nullptr };
     Node* m_head { nullptr };
     Node* m_tail { nullptr };
     Node* m_tail { nullptr };
 };
 };

+ 1 - 0
AK/Tests/CMakeLists.txt

@@ -11,6 +11,7 @@ set(AK_TEST_SOURCES
     TestCircularDuplexStream.cpp
     TestCircularDuplexStream.cpp
     TestCircularQueue.cpp
     TestCircularQueue.cpp
     TestDistinctNumeric.cpp
     TestDistinctNumeric.cpp
+    TestDoublyLinkedList.cpp
     TestEndian.cpp
     TestEndian.cpp
     TestFind.cpp
     TestFind.cpp
     TestFormat.cpp
     TestFormat.cpp

+ 65 - 0
AK/Tests/TestDoublyLinkedList.cpp

@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2021, the SerenityOS developers.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ *    list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <AK/TestSuite.h>
+
+#include <AK/DoublyLinkedList.h>
+
+static DoublyLinkedList<int> make_list()
+{
+    DoublyLinkedList<int> list {};
+    list.append(0);
+    list.append(1);
+    list.append(2);
+    list.append(3);
+    list.append(4);
+    list.append(5);
+    list.append(6);
+    list.append(7);
+    list.append(8);
+    list.append(9);
+    return list;
+}
+
+TEST_CASE(should_find_mutable)
+{
+    auto sut = make_list();
+
+    EXPECT_EQ(4, *sut.find(4));
+
+    EXPECT_EQ(sut.end(), sut.find(42));
+}
+
+TEST_CASE(should_find_const)
+{
+    const auto sut = make_list();
+
+    EXPECT_EQ(4, *sut.find(4));
+
+    EXPECT_EQ(sut.end(), sut.find(42));
+}
+
+TEST_MAIN(DoublyLinkedList)

+ 1 - 1
Kernel/Devices/Device.h

@@ -75,7 +75,7 @@ public:
         {
         {
             ScopedSpinLock lock(m_requests_lock);
             ScopedSpinLock lock(m_requests_lock);
             was_empty = m_requests.is_empty();
             was_empty = m_requests.is_empty();
-            m_requests.append(request);
+            m_requests.append(RefPtr<AsyncDeviceRequest>(request));
         }
         }
         if (was_empty)
         if (was_empty)
             request->do_start({});
             request->do_start({});