diff options
author | Lenny Maiorani <lenny@colorado.edu> | 2020-12-23 11:35:20 -0700 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-01-11 19:45:05 +0100 |
commit | f99d1d3bd7d6f752c490c358217c360b7e0bda80 (patch) | |
tree | f06dfaaec5128b6a3844b29f316e0ee20235caf0 | |
parent | 4333a9a8d6d9b98e0923e09e06deb41ee5bcd5b7 (diff) | |
download | serenity-f99d1d3bd7d6f752c490c358217c360b7e0bda80.zip |
Vector: Implement `find`, `find_if`, `find_first_matching` in terms of `AK::find*`
Problem:
- The implementation of `find` is coupled to the implementation of `Vector`.
- `Vector::find` takes the predicate by value which might be expensive.
Solution:
- Decouple the implementation of `find` from `Vector` by using a
generic `find` algorithm.
- Change the name of `find` with a predicate to `find_if` so that a
binding reference can be used and the predicate can be forwarded to
avoid copies.
- Change all the `find(pred)` call sites to use `find_if`.
-rw-r--r-- | AK/Tests/TestVector.cpp | 26 | ||||
-rw-r--r-- | AK/Vector.h | 31 | ||||
-rw-r--r-- | Applications/Spreadsheet/Cell.cpp | 2 | ||||
-rw-r--r-- | Applications/Spreadsheet/Readers/XSV.cpp | 2 | ||||
-rw-r--r-- | Applications/SystemMonitor/DevicesModel.cpp | 2 | ||||
-rw-r--r-- | DevTools/HackStudio/Debugger/VariablesModel.cpp | 4 | ||||
-rw-r--r-- | Libraries/LibCore/ArgsParser.cpp | 2 | ||||
-rw-r--r-- | Libraries/LibMarkdown/Text.cpp | 2 | ||||
-rw-r--r-- | Libraries/LibTLS/ClientHandshake.cpp | 2 | ||||
-rw-r--r-- | Services/WindowServer/MenuManager.cpp | 4 |
10 files changed, 48 insertions, 29 deletions
diff --git a/AK/Tests/TestVector.cpp b/AK/Tests/TestVector.cpp index 8278208428..a581815bec 100644 --- a/AK/Tests/TestVector.cpp +++ b/AK/Tests/TestVector.cpp @@ -394,4 +394,30 @@ TEST_CASE(should_compare_vectors_of_different_sizes) EXPECT(a != b); } +TEST_CASE(should_find_value) +{ + AK::Vector<int> v { 1, 2, 3, 4, 0, 6, 7, 8, 0, 0 }; + + const auto expected = v.begin() + 4; + + EXPECT_EQ(expected, v.find(0)); +} + +TEST_CASE(should_find_predicate) +{ + AK::Vector<int> v { 1, 2, 3, 4, 0, 6, 7, 8, 0, 0 }; + + const auto expected = v.begin() + 4; + + EXPECT_EQ(expected, v.find_if([](const auto v) { return v == 0; })); +} + +TEST_CASE(should_find_index) +{ + AK::Vector<int> v { 1, 2, 3, 4, 0, 6, 7, 8, 0, 0 }; + + EXPECT_EQ(4u, v.find_first_index(0).value()); + EXPECT(!v.find_first_index(42).has_value()); +} + TEST_MAIN(Vector) diff --git a/AK/Vector.h b/AK/Vector.h index 362ad9db3b..8ea8042852 100644 --- a/AK/Vector.h +++ b/AK/Vector.h @@ -27,6 +27,7 @@ #pragma once #include <AK/Assertions.h> +#include <AK/Find.h> #include <AK/Forward.h> #include <AK/Iterator.h> #include <AK/Optional.h> @@ -538,41 +539,33 @@ public: ConstIterator end() const { return ConstIterator::end(*this); } Iterator end() { return Iterator::end(*this); } - template<typename Finder> - ConstIterator find(Finder finder) const + template<typename TUnaryPredicate> + ConstIterator find_if(TUnaryPredicate&& finder) const { - for (size_t i = 0; i < m_size; ++i) { - if (finder(at(i))) - return ConstIterator(*this, i); - } - return end(); + return AK::find_if(begin(), end(), forward<TUnaryPredicate>(finder)); } - template<typename Finder> - Iterator find(Finder finder) + template<typename TUnaryPredicate> + Iterator find_if(TUnaryPredicate&& finder) { - for (size_t i = 0; i < m_size; ++i) { - if (finder(at(i))) - return Iterator { *this, i }; - } - return end(); + return AK::find_if(begin(), end(), forward<TUnaryPredicate>(finder)); } ConstIterator find(const T& value) const { - return find([&](auto& other) { return Traits<T>::equals(value, other); }); + return AK::find(begin(), end(), value); } Iterator find(const T& value) { - return find([&](auto& other) { return Traits<T>::equals(value, other); }); + return AK::find(begin(), end(), value); } Optional<size_t> find_first_index(const T& value) { - for (size_t i = 0; i < m_size; ++i) { - if (Traits<T>::equals(value, at(i))) - return i; + if (const auto index = AK::find_index(begin(), end(), value); + index < size()) { + return index; } return {}; } diff --git a/Applications/Spreadsheet/Cell.cpp b/Applications/Spreadsheet/Cell.cpp index 2192ce3d66..d2f511753f 100644 --- a/Applications/Spreadsheet/Cell.cpp +++ b/Applications/Spreadsheet/Cell.cpp @@ -187,7 +187,7 @@ void Cell::reference_from(Cell* other) if (!other || other == this) return; - if (!m_referencing_cells.find([other](auto& ptr) { return ptr.ptr() == other; }).is_end()) + if (!m_referencing_cells.find_if([other](const auto& ptr) { return ptr.ptr() == other; }).is_end()) return; m_referencing_cells.append(other->make_weak_ptr()); diff --git a/Applications/Spreadsheet/Readers/XSV.cpp b/Applications/Spreadsheet/Readers/XSV.cpp index d87c86748e..99a61b0abc 100644 --- a/Applications/Spreadsheet/Readers/XSV.cpp +++ b/Applications/Spreadsheet/Readers/XSV.cpp @@ -244,7 +244,7 @@ XSV::Field XSV::read_one_unquoted_field() StringView XSV::Row::operator[](StringView name) const { ASSERT(!m_xsv.m_names.is_empty()); - auto it = m_xsv.m_names.find([&](auto&& entry) { return name == entry; }); + auto it = m_xsv.m_names.find_if([&](const auto& entry) { return name == entry; }); ASSERT(!it.is_end()); return (*this)[it.index()]; diff --git a/Applications/SystemMonitor/DevicesModel.cpp b/Applications/SystemMonitor/DevicesModel.cpp index 6957c666b8..f0692cdd24 100644 --- a/Applications/SystemMonitor/DevicesModel.cpp +++ b/Applications/SystemMonitor/DevicesModel.cpp @@ -182,7 +182,7 @@ void DevicesModel::update() unsigned _major = major(statbuf.st_rdev); unsigned _minor = minor(statbuf.st_rdev); - auto it = m_devices.find([_major, _minor](auto& device_info) { + auto it = m_devices.find_if([_major, _minor](const auto& device_info) { return device_info.major == _major && device_info.minor == _minor; }); if (it != m_devices.end()) { diff --git a/DevTools/HackStudio/Debugger/VariablesModel.cpp b/DevTools/HackStudio/Debugger/VariablesModel.cpp index b166154d25..425129046c 100644 --- a/DevTools/HackStudio/Debugger/VariablesModel.cpp +++ b/DevTools/HackStudio/Debugger/VariablesModel.cpp @@ -80,7 +80,7 @@ static String variable_value_as_string(const Debug::DebugInfo::VariableInfo& var if (variable.is_enum_type()) { auto value = Debugger::the().session()->peek((u32*)variable_address); ASSERT(value.has_value()); - auto it = variable.type->members.find([enumerator_value = value.value()](auto& enumerator) { + auto it = variable.type->members.find_if([&enumerator_value = value.value()](const auto& enumerator) { return enumerator->constant_data.as_u32 == enumerator_value; }); ASSERT(!it.is_end()); @@ -116,7 +116,7 @@ static Optional<u32> string_to_variable_value(const StringView& string_value, co if (string_value.starts_with(prefix_string)) string_to_use = string_value.substring_view(prefix_string.length(), string_value.length() - prefix_string.length()); - auto it = variable.type->members.find([string_to_use](auto& enumerator) { + auto it = variable.type->members.find_if([string_to_use](const auto& enumerator) { return enumerator->name == string_to_use; }); diff --git a/Libraries/LibCore/ArgsParser.cpp b/Libraries/LibCore/ArgsParser.cpp index 16583b9012..121272e38b 100644 --- a/Libraries/LibCore/ArgsParser.cpp +++ b/Libraries/LibCore/ArgsParser.cpp @@ -109,7 +109,7 @@ bool ArgsParser::parse(int argc, char** argv, bool exit_on_failure) index_of_found_long_option = -1; } else { // It was a short option, look it up. - auto it = m_options.find([c](auto& opt) { return c == opt.short_name; }); + auto it = m_options.find_if([c](auto& opt) { return c == opt.short_name; }); ASSERT(!it.is_end()); found_option = &*it; } diff --git a/Libraries/LibMarkdown/Text.cpp b/Libraries/LibMarkdown/Text.cpp index ddb0ea2d81..9361e05d60 100644 --- a/Libraries/LibMarkdown/Text.cpp +++ b/Libraries/LibMarkdown/Text.cpp @@ -69,7 +69,7 @@ String Text::render_to_html() const { "b", &Style::strong }, { "code", &Style::code } }; - auto it = open_tags.find([&](const String& open_tag) { + auto it = open_tags.find_if([&](const String& open_tag) { if (open_tag == "a" && current_style.href != span.style.href) return true; if (open_tag == "img" && current_style.img != span.style.img) diff --git a/Libraries/LibTLS/ClientHandshake.cpp b/Libraries/LibTLS/ClientHandshake.cpp index 1632bf6a76..b1fbaa2391 100644 --- a/Libraries/LibTLS/ClientHandshake.cpp +++ b/Libraries/LibTLS/ClientHandshake.cpp @@ -398,7 +398,7 @@ ssize_t TLSv12::handle_payload(ReadonlyBytes vbuffer) } payload_res = handle_certificate(buffer.slice(1, payload_size)); if (m_context.certificates.size()) { - auto it = m_context.certificates.find([&](auto& cert) { return cert.is_valid(); }); + auto it = m_context.certificates.find_if([](const auto& cert) { return cert.is_valid(); }); if (it.is_end()) { // no valid certificates diff --git a/Services/WindowServer/MenuManager.cpp b/Services/WindowServer/MenuManager.cpp index 28639db98f..ba2ba789f9 100644 --- a/Services/WindowServer/MenuManager.cpp +++ b/Services/WindowServer/MenuManager.cpp @@ -162,7 +162,7 @@ void MenuManager::event(Core::Event& event) if (event.type() == Event::KeyDown) { if (key_event.key() == Key_Left) { - auto it = m_open_menu_stack.find([&](auto& other) { return m_current_menu == other.ptr(); }); + auto it = m_open_menu_stack.find_if([&](const auto& other) { return m_current_menu == other.ptr(); }); ASSERT(!it.is_end()); // Going "back" a menu should be the previous menu in the stack @@ -390,7 +390,7 @@ void MenuManager::open_menu(Menu& menu, bool as_current_menu) menu.menu_window()->set_visible(true); } - if (m_open_menu_stack.find([&menu](auto& other) { return &menu == other.ptr(); }).is_end()) + if (m_open_menu_stack.find_if([&menu](auto& other) { return &menu == other.ptr(); }).is_end()) m_open_menu_stack.append(menu); if (as_current_menu || !current_menu()) { |