From 3bdaed501e13a8b2dcdbbe2b5f4f68ffa22db59e Mon Sep 17 00:00:00 2001 From: Max Wipfli Date: Thu, 1 Jul 2021 15:01:29 +0200 Subject: [PATCH] AK+Everywhere: Remove StringView::find_{first,last}_of(char) methods This removes StringView::find_first_of(char) and find_last_of(char) and replaces all its usages with find and find_last respectively. This is because those two methods are functionally equivalent. find_{first,last}_of should only be used if searching for multiple different characters, which is never the case with the char argument. This also adds the [[nodiscard]] to the remaining find_{first,last}_of methods. --- AK/LexicalPath.cpp | 4 +- AK/StringView.cpp | 17 ----- AK/StringView.h | 9 +-- Tests/AK/TestStringView.cpp | 67 +++++++++---------- Userland/Applications/Browser/CookieJar.cpp | 2 +- .../Applications/SoundPlayer/M3UParser.cpp | 2 +- .../Applications/Spreadsheet/Spreadsheet.cpp | 2 +- Userland/Services/LaunchServer/Launcher.cpp | 2 +- Userland/Services/WindowServer/Cursor.cpp | 2 +- Userland/Utilities/pro.cpp | 2 +- 10 files changed, 41 insertions(+), 68 deletions(-) diff --git a/AK/LexicalPath.cpp b/AK/LexicalPath.cpp index 9d6ee4e726..f1bd4c5a72 100644 --- a/AK/LexicalPath.cpp +++ b/AK/LexicalPath.cpp @@ -29,7 +29,7 @@ LexicalPath::LexicalPath(String path) m_parts = m_string.split_view('/'); - auto last_slash_index = m_string.view().find_last_of('/'); + auto last_slash_index = m_string.view().find_last('/'); if (!last_slash_index.has_value()) { // The path contains a single part and is not absolute. m_dirname = "."sv m_dirname = { &s_single_dot, 1 }; @@ -47,7 +47,7 @@ LexicalPath::LexicalPath(String path) m_basename = m_parts.last(); } - auto last_dot_index = m_basename.find_last_of('.'); + auto last_dot_index = m_basename.find_last('.'); // NOTE: if the dot index is 0, this means we have ".foo", it's not an extension, as the title would then be "". if (last_dot_index.has_value() && *last_dot_index != 0) { m_title = m_basename.substring_view(0, *last_dot_index); diff --git a/AK/StringView.cpp b/AK/StringView.cpp index 9960d66707..5fe62fe8b8 100644 --- a/AK/StringView.cpp +++ b/AK/StringView.cpp @@ -238,14 +238,6 @@ bool StringView::operator==(const String& string) const return !__builtin_memcmp(m_characters, string.characters(), m_length); } -Optional StringView::find_first_of(char c) const -{ - if (const auto location = AK::find(begin(), end(), c); location != end()) { - return location.index(); - } - return {}; -} - Optional StringView::find_first_of(const StringView& view) const { if (const auto location = AK::find_if(begin(), end(), @@ -261,15 +253,6 @@ Optional StringView::find_first_of(const StringView& view) const return {}; } -Optional StringView::find_last_of(char c) const -{ - for (size_t pos = m_length; pos != 0; --pos) { - if (m_characters[pos - 1] == c) - return pos - 1; - } - return {}; -} - Optional StringView::find_last_of(const StringView& view) const { for (size_t pos = m_length; pos != 0; --pos) { diff --git a/AK/StringView.h b/AK/StringView.h index 1c4da71248..86a57cb94f 100644 --- a/AK/StringView.h +++ b/AK/StringView.h @@ -86,17 +86,14 @@ public: [[nodiscard]] String to_lowercase_string() const; [[nodiscard]] String to_uppercase_string() const; - Optional find_first_of(char) const; - Optional find_first_of(const StringView&) const; - - Optional find_last_of(char) const; - Optional find_last_of(const StringView&) const; - [[nodiscard]] Optional find(char needle, size_t start = 0) const { return StringUtils::find(*this, needle, start); } [[nodiscard]] Optional find(StringView const& needle, size_t start = 0) const { return StringUtils::find(*this, needle, start); } [[nodiscard]] Optional find_last(char needle) const { return StringUtils::find_last(*this, needle); } // FIXME: Implement find_last(StringView const&) for API symmetry. + [[nodiscard]] Optional find_first_of(StringView const&) const; + [[nodiscard]] Optional find_last_of(StringView const&) const; + [[nodiscard]] constexpr StringView substring_view(size_t start, size_t length) const { if (!is_constant_evaluated()) diff --git a/Tests/AK/TestStringView.cpp b/Tests/AK/TestStringView.cpp index 8f7785dce3..fa079c9ebb 100644 --- a/Tests/AK/TestStringView.cpp +++ b/Tests/AK/TestStringView.cpp @@ -104,52 +104,45 @@ TEST_CASE(lines) EXPECT_EQ(test_string_vector.at(2).is_empty(), true); } +TEST_CASE(find) +{ + auto test_string_view = "aabbcc_xy_ccbbaa"sv; + EXPECT_EQ(test_string_view.find('b'), 2U); + EXPECT_EQ(test_string_view.find('_'), 6U); + EXPECT_EQ(test_string_view.find('n').has_value(), false); +} + +TEST_CASE(find_last) +{ + auto test_string_view = "aabbcc_xy_ccbbaa"sv; + EXPECT_EQ(test_string_view.find_last('b'), 13U); + EXPECT_EQ(test_string_view.find_last('_'), 9U); + EXPECT_EQ(test_string_view.find_last('3').has_value(), false); + + test_string_view = "/"sv; + EXPECT_EQ(test_string_view.find_last('/'), 0U); +} + TEST_CASE(find_first_of) { - String test_string = "aabbcc_xy_ccbbaa"; - StringView test_string_view = test_string.view(); - - EXPECT_EQ(test_string_view.find_first_of('b').has_value(), true); - EXPECT_EQ(test_string_view.find_first_of('b').value(), 2U); - - EXPECT_EQ(test_string_view.find_first_of('_').has_value(), true); - EXPECT_EQ(test_string_view.find_first_of('_').value(), 6U); - - EXPECT_EQ(test_string_view.find_first_of("bc").has_value(), true); - EXPECT_EQ(test_string_view.find_first_of("bc").value(), 2U); - - EXPECT_EQ(test_string_view.find_first_of("yx").has_value(), true); - EXPECT_EQ(test_string_view.find_first_of("yx").value(), 7U); - - EXPECT_EQ(test_string_view.find_first_of('n').has_value(), false); + auto test_string_view = "aabbcc_xy_ccbbaa"sv; + EXPECT_EQ(test_string_view.find_first_of("bc"), 2U); + EXPECT_EQ(test_string_view.find_first_of("yx"), 7U); EXPECT_EQ(test_string_view.find_first_of("defg").has_value(), false); + + test_string_view = "/"sv; + EXPECT_EQ(test_string_view.find_first_of("/"), 0U); } TEST_CASE(find_last_of) { - String test_string = "aabbcc_xy_ccbbaa"; - StringView test_string_view = test_string.view(); - - EXPECT_EQ(test_string_view.find_last_of('b').has_value(), true); - EXPECT_EQ(test_string_view.find_last_of('b').value(), 13U); - - EXPECT_EQ(test_string_view.find_last_of('_').has_value(), true); - EXPECT_EQ(test_string_view.find_last_of('_').value(), 9U); - - EXPECT_EQ(test_string_view.find_last_of("bc").has_value(), true); - EXPECT_EQ(test_string_view.find_last_of("bc").value(), 13U); - - EXPECT_EQ(test_string_view.find_last_of("yx").has_value(), true); - EXPECT_EQ(test_string_view.find_last_of("yx").value(), 8U); - - EXPECT_EQ(test_string_view.find_last_of('3').has_value(), false); + auto test_string_view = "aabbcc_xy_ccbbaa"sv; + EXPECT_EQ(test_string_view.find_last_of("bc"), 13U); + EXPECT_EQ(test_string_view.find_last_of("yx"), 8U); EXPECT_EQ(test_string_view.find_last_of("fghi").has_value(), false); - test_string_view = "/"; - EXPECT_EQ(test_string_view.find_last_of('/').has_value(), true); - EXPECT_EQ(test_string_view.find_last_of('/').value(), 0U); - EXPECT_EQ(test_string_view.find_last_of("/").has_value(), true); - EXPECT_EQ(test_string_view.find_last_of("/").value(), 0U); + test_string_view = "/"sv; + EXPECT_EQ(test_string_view.find_last_of("/"), 0U); } TEST_CASE(split_view) diff --git a/Userland/Applications/Browser/CookieJar.cpp b/Userland/Applications/Browser/CookieJar.cpp index 2cdab6aea6..4a27cc53c9 100644 --- a/Userland/Applications/Browser/CookieJar.cpp +++ b/Userland/Applications/Browser/CookieJar.cpp @@ -142,7 +142,7 @@ String CookieJar::default_path(const URL& url) return "/"; StringView uri_path_view = uri_path; - std::size_t last_separator = uri_path_view.find_last_of('/').value(); + size_t last_separator = uri_path_view.find_last('/').value(); // 3. If the uri-path contains no more than one %x2F ("/") character, output %x2F ("/") and skip the remaining step. if (last_separator == 0) diff --git a/Userland/Applications/SoundPlayer/M3UParser.cpp b/Userland/Applications/SoundPlayer/M3UParser.cpp index e7c3e2ffda..b8267ec624 100644 --- a/Userland/Applications/SoundPlayer/M3UParser.cpp +++ b/Userland/Applications/SoundPlayer/M3UParser.cpp @@ -64,7 +64,7 @@ NonnullOwnPtr> M3UParser::parse(bool include_extended_info) if (line.starts_with('#') && has_exteded_info_tag) { if (line.starts_with("#EXTINF")) { auto data = line.substring_view(8); - auto separator = data.find_first_of(','); + auto separator = data.find(','); VERIFY(separator.has_value()); auto seconds = data.substring_view(0, separator.value()); VERIFY(!seconds.is_whitespace() && !seconds.is_null() && !seconds.is_empty()); diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.cpp b/Userland/Applications/Spreadsheet/Spreadsheet.cpp index e845914d5e..95ddffcbca 100644 --- a/Userland/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Userland/Applications/Spreadsheet/Spreadsheet.cpp @@ -91,7 +91,7 @@ static size_t convert_from_string(StringView str, unsigned base = 26, StringView size_t value = 0; for (size_t i = str.length(); i > 0; --i) { - auto digit_value = map.find_first_of(str[i - 1]).value_or(0); + auto digit_value = map.find(str[i - 1]).value_or(0); // NOTE: Refer to the note in `String::bijective_base_from()'. if (i == str.length() && str.length() > 1) ++digit_value; diff --git a/Userland/Services/LaunchServer/Launcher.cpp b/Userland/Services/LaunchServer/Launcher.cpp index 96985375b2..29cf55947b 100644 --- a/Userland/Services/LaunchServer/Launcher.cpp +++ b/Userland/Services/LaunchServer/Launcher.cpp @@ -26,7 +26,7 @@ static bool spawn(String executable, const Vector& arguments); String Handler::name_from_executable(const StringView& executable) { - auto separator = executable.find_last_of('/'); + auto separator = executable.find_last('/'); if (separator.has_value()) { auto start = separator.value() + 1; return executable.substring_view(start, executable.length() - start); diff --git a/Userland/Services/WindowServer/Cursor.cpp b/Userland/Services/WindowServer/Cursor.cpp index 2a192b1fa3..85df6a2022 100644 --- a/Userland/Services/WindowServer/Cursor.cpp +++ b/Userland/Services/WindowServer/Cursor.cpp @@ -15,7 +15,7 @@ CursorParams CursorParams::parse_from_filename(const StringView& cursor_path, co { LexicalPath path(cursor_path); auto file_title = path.title(); - auto last_dot_in_title = StringView(file_title).find_last_of('.'); + auto last_dot_in_title = file_title.find_last('.'); if (!last_dot_in_title.has_value() || last_dot_in_title.value() == 0) { // No encoded params in filename. Not an error, we'll just use defaults return { default_hotspot }; diff --git a/Userland/Utilities/pro.cpp b/Userland/Utilities/pro.cpp index 16efa987d1..7556eb7a17 100644 --- a/Userland/Utilities/pro.cpp +++ b/Userland/Utilities/pro.cpp @@ -163,7 +163,7 @@ int main(int argc, char** argv) .value_name = "header-value", .accept_value = [&](auto* s) { StringView header { s }; - auto split = header.find_first_of(':'); + auto split = header.find(':'); if (!split.has_value()) return false; request_headers.set(header.substring_view(0, split.value()), header.substring_view(split.value() + 1));