From 1b2364846fe30cd6e6ce0e801f031107a1846db3 Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Sun, 10 Jan 2021 11:27:41 -0700 Subject: [PATCH] SinglyLinkedList: Implement `find` in terms of `AK::find` Problem: - The implementation of `find` is coupled to the implementation of `SinglyLinkedList`. Solution: - Decouple the implementation of `find` from the class by using a generic `find` algorithm. --- AK/DoublyLinkedList.h | 6 ++- AK/SinglyLinkedList.h | 29 ++++------- AK/SinglyLinkedListWithCount.h | 13 ++--- AK/Tests/CMakeLists.txt | 1 + AK/Tests/TestSinglyLinkedList.cpp | 83 +++++++++++++++++++++++++++++++ Applications/Piano/Track.cpp | 2 +- Kernel/Devices/Device.h | 2 +- 7 files changed, 106 insertions(+), 30 deletions(-) create mode 100644 AK/Tests/TestSinglyLinkedList.cpp diff --git a/AK/DoublyLinkedList.h b/AK/DoublyLinkedList.h index e679a3e37d5..166fc9f4f40 100644 --- a/AK/DoublyLinkedList.h +++ b/AK/DoublyLinkedList.h @@ -65,7 +65,8 @@ private: explicit Node(U&& v) : value(forward(v)) { - static_assert(IsSame::value); + static_assert( + requires { T(v); }, "Conversion operator is missing."); } T value; Node* next { nullptr }; @@ -113,7 +114,8 @@ public: template void append(U&& value) { - static_assert(IsSame::value); + static_assert( + requires { T(value); }, "Conversion operator is missing."); auto* node = new Node(forward(value)); if (!m_head) { ASSERT(!m_tail); diff --git a/AK/SinglyLinkedList.h b/AK/SinglyLinkedList.h index d41e9039d1e..8f2430c1f7e 100644 --- a/AK/SinglyLinkedList.h +++ b/AK/SinglyLinkedList.h @@ -27,6 +27,7 @@ #pragma once #include +#include #include #include #include @@ -166,38 +167,26 @@ public: ConstIterator begin() const { return ConstIterator(m_head); } ConstIterator end() const { return {}; } - template - ConstIterator find(Finder finder) const + template + ConstIterator find_if(TUnaryPredicate&& pred) const { - Node* prev = nullptr; - for (auto* node = m_head; node; node = node->next) { - if (finder(node->value)) - return ConstIterator(node, prev); - prev = node; - } - return end(); + return AK::find_if(begin(), end(), forward(pred)); } - template - Iterator find(Finder finder) + template + Iterator find_if(TUnaryPredicate&& pred) { - Node* prev = nullptr; - for (auto* node = m_head; node; node = node->next) { - if (finder(node->value)) - return Iterator(node, prev); - prev = node; - } - return end(); + return AK::find_if(begin(), end(), forward(pred)); } ConstIterator find(const T& value) const { - return find([&](auto& other) { return Traits::equals(value, other); }); + return find_if([&](auto& other) { return Traits::equals(value, other); }); } Iterator find(const T& value) { - return find([&](auto& other) { return Traits::equals(value, other); }); + return find_if([&](auto& other) { return Traits::equals(value, other); }); } void remove(Iterator iterator) diff --git a/AK/SinglyLinkedListWithCount.h b/AK/SinglyLinkedListWithCount.h index ec9d0196c9b..8d3eb804272 100644 --- a/AK/SinglyLinkedListWithCount.h +++ b/AK/SinglyLinkedListWithCount.h @@ -27,6 +27,7 @@ #pragma once #include +#include namespace AK { @@ -106,16 +107,16 @@ public: ConstIterator begin() const { return List::begin(); } ConstIterator end() const { return List::end(); } - template - ConstIterator find(Finder finder) const + template + ConstIterator find(TUnaryPredicate&& pred) const { - return List::find(finder); + return List::find_if(forward(pred)); } - template - Iterator find(Finder finder) + template + Iterator find(TUnaryPredicate&& pred) { - return List::find(finder); + return List::find_if(forward(pred)); } ConstIterator find(const T& value) const diff --git a/AK/Tests/CMakeLists.txt b/AK/Tests/CMakeLists.txt index 4518b301403..303fa219a13 100644 --- a/AK/Tests/CMakeLists.txt +++ b/AK/Tests/CMakeLists.txt @@ -31,6 +31,7 @@ set(AK_TEST_SOURCES TestQueue.cpp TestQuickSort.cpp TestRefPtr.cpp + TestSinglyLinkedList.cpp TestSourceGenerator.cpp TestSpan.cpp TestString.cpp diff --git a/AK/Tests/TestSinglyLinkedList.cpp b/AK/Tests/TestSinglyLinkedList.cpp new file mode 100644 index 00000000000..009187e60ba --- /dev/null +++ b/AK/Tests/TestSinglyLinkedList.cpp @@ -0,0 +1,83 @@ +/* + * 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 + +#include + +static SinglyLinkedList make_list() +{ + SinglyLinkedList 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_mutable_with_predicate) +{ + auto sut = make_list(); + + EXPECT_EQ(4, *sut.find_if([](const auto v) { return v == 4; })); + + EXPECT_EQ(sut.end(), sut.find_if([](const auto v) { return v == 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_CASE(should_find_const_with_predicate) +{ + const auto sut = make_list(); + + EXPECT_EQ(4, *sut.find_if([](const auto v) { return v == 4; })); + + EXPECT_EQ(sut.end(), sut.find_if([](const auto v) { return v == 42; })); +} + +TEST_MAIN(SinglyLinkedList) diff --git a/Applications/Piano/Track.cpp b/Applications/Piano/Track.cpp index 8bbdf5474fa..61d4351d061 100644 --- a/Applications/Piano/Track.cpp +++ b/Applications/Piano/Track.cpp @@ -266,7 +266,7 @@ void Track::set_note(int note, Switch switch_note) void Track::sync_roll(int note) { - auto it = m_roll_notes[note].find([&](auto& roll_note) { return roll_note.off_sample > m_time; }); + auto it = m_roll_notes[note].find_if([&](auto& roll_note) { return roll_note.off_sample > m_time; }); if (it.is_end()) m_roll_iters[note] = m_roll_notes[note].begin(); else diff --git a/Kernel/Devices/Device.h b/Kernel/Devices/Device.h index b97ed35b254..1ef8bcfa89e 100644 --- a/Kernel/Devices/Device.h +++ b/Kernel/Devices/Device.h @@ -75,7 +75,7 @@ public: { ScopedSpinLock lock(m_requests_lock); was_empty = m_requests.is_empty(); - m_requests.append(RefPtr(request)); + m_requests.append(request); } if (was_empty) request->do_start({});