mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2024-11-22 07:30:19 +00:00
AK+Kernel: Unify Traits<T>::equals()'s argument order on different types
There was a small mishmash of argument order, as seen on the table: | Traits<T>::equals(U, T) | Traits<T>::equals(T, U) ============= | ======================= | ======================= uses equals() | HashMap | Vector, HashTable defines equals() | *String[^1] | ByteBuffer [^1]: String, DeprecatedString, their Fly-type equivalents and KString. This mostly meant that you couldn't use a StringView for finding a value in Vector<String>. I'm changing the order of arguments to make the trait type itself first (`Traits<T>::equals(T, U)`), as I think it's more expected and makes us more consistent with the rest of the functions that put the stored type first (like StringUtils functions and binary_serach). I've also renamed the variable name "other" in find functions to "entry" to give more importance to the value. With this change, each of the following lines will now compile successfully: Vector<String>().contains_slow("WHF!"sv); HashTable<String>().contains("WHF!"sv); HashMap<ByteBuffer, int>().contains("WHF!"sv.bytes());
This commit is contained in:
parent
5ff7448fee
commit
e575ee4462
Notes:
sideshowbarker
2024-07-17 08:38:37 +09:00
Author: https://github.com/krkk Commit: https://github.com/SerenityOS/serenity/commit/e575ee4462 Pull-request: https://github.com/SerenityOS/serenity/pull/20722
7 changed files with 26 additions and 16 deletions
|
@ -26,6 +26,7 @@ template<typename TEndIterator, IteratorPairWith<TEndIterator> TIterator, typena
|
||||||
template<typename TEndIterator, IteratorPairWith<TEndIterator> TIterator, typename T>
|
template<typename TEndIterator, IteratorPairWith<TEndIterator> TIterator, typename T>
|
||||||
[[nodiscard]] constexpr TIterator find(TIterator first, TEndIterator last, T const& value)
|
[[nodiscard]] constexpr TIterator find(TIterator first, TEndIterator last, T const& value)
|
||||||
{
|
{
|
||||||
|
// FIXME: Use the iterator's trait type, and swap arguments in equals call.
|
||||||
return find_if(first, last, [&](auto const& v) { return Traits<T>::equals(value, v); });
|
return find_if(first, last, [&](auto const& v) { return Traits<T>::equals(value, v); });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -33,6 +34,7 @@ template<typename TEndIterator, IteratorPairWith<TEndIterator> TIterator, typena
|
||||||
[[nodiscard]] constexpr size_t find_index(TIterator first, TEndIterator last, T const& value)
|
[[nodiscard]] constexpr size_t find_index(TIterator first, TEndIterator last, T const& value)
|
||||||
requires(requires(TIterator it) { it.index(); })
|
requires(requires(TIterator it) { it.index(); })
|
||||||
{
|
{
|
||||||
|
// FIXME: Use the iterator's trait type, and swap arguments in equals call.
|
||||||
return find_if(first, last, [&](auto const& v) { return Traits<T>::equals(value, v); }).index();
|
return find_if(first, last, [&](auto const& v) { return Traits<T>::equals(value, v); }).index();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -98,7 +98,7 @@ public:
|
||||||
[[nodiscard]] IteratorType end() { return m_table.end(); }
|
[[nodiscard]] IteratorType end() { return m_table.end(); }
|
||||||
[[nodiscard]] IteratorType find(K const& key)
|
[[nodiscard]] IteratorType find(K const& key)
|
||||||
{
|
{
|
||||||
return m_table.find(KeyTraits::hash(key), [&](auto& entry) { return KeyTraits::equals(key, entry.key); });
|
return m_table.find(KeyTraits::hash(key), [&](auto& entry) { return KeyTraits::equals(entry.key, key); });
|
||||||
}
|
}
|
||||||
template<typename TUnaryPredicate>
|
template<typename TUnaryPredicate>
|
||||||
[[nodiscard]] IteratorType find(unsigned hash, TUnaryPredicate predicate)
|
[[nodiscard]] IteratorType find(unsigned hash, TUnaryPredicate predicate)
|
||||||
|
@ -110,7 +110,7 @@ public:
|
||||||
[[nodiscard]] ConstIteratorType end() const { return m_table.end(); }
|
[[nodiscard]] ConstIteratorType end() const { return m_table.end(); }
|
||||||
[[nodiscard]] ConstIteratorType find(K const& key) const
|
[[nodiscard]] ConstIteratorType find(K const& key) const
|
||||||
{
|
{
|
||||||
return m_table.find(KeyTraits::hash(key), [&](auto& entry) { return KeyTraits::equals(key, entry.key); });
|
return m_table.find(KeyTraits::hash(key), [&](auto& entry) { return KeyTraits::equals(entry.key, key); });
|
||||||
}
|
}
|
||||||
template<typename TUnaryPredicate>
|
template<typename TUnaryPredicate>
|
||||||
[[nodiscard]] ConstIteratorType find(unsigned hash, TUnaryPredicate predicate) const
|
[[nodiscard]] ConstIteratorType find(unsigned hash, TUnaryPredicate predicate) const
|
||||||
|
@ -121,13 +121,13 @@ public:
|
||||||
template<Concepts::HashCompatible<K> Key>
|
template<Concepts::HashCompatible<K> Key>
|
||||||
requires(IsSame<KeyTraits, Traits<K>>) [[nodiscard]] IteratorType find(Key const& key)
|
requires(IsSame<KeyTraits, Traits<K>>) [[nodiscard]] IteratorType find(Key const& key)
|
||||||
{
|
{
|
||||||
return m_table.find(Traits<Key>::hash(key), [&](auto& entry) { return Traits<K>::equals(key, entry.key); });
|
return m_table.find(Traits<Key>::hash(key), [&](auto& entry) { return Traits<K>::equals(entry.key, key); });
|
||||||
}
|
}
|
||||||
|
|
||||||
template<Concepts::HashCompatible<K> Key>
|
template<Concepts::HashCompatible<K> Key>
|
||||||
requires(IsSame<KeyTraits, Traits<K>>) [[nodiscard]] ConstIteratorType find(Key const& key) const
|
requires(IsSame<KeyTraits, Traits<K>>) [[nodiscard]] ConstIteratorType find(Key const& key) const
|
||||||
{
|
{
|
||||||
return m_table.find(Traits<Key>::hash(key), [&](auto& entry) { return Traits<K>::equals(key, entry.key); });
|
return m_table.find(Traits<Key>::hash(key), [&](auto& entry) { return Traits<K>::equals(entry.key, key); });
|
||||||
}
|
}
|
||||||
|
|
||||||
ErrorOr<void> try_ensure_capacity(size_t capacity) { return m_table.try_ensure_capacity(capacity); }
|
ErrorOr<void> try_ensure_capacity(size_t capacity) { return m_table.try_ensure_capacity(capacity); }
|
||||||
|
|
|
@ -317,7 +317,7 @@ public:
|
||||||
|
|
||||||
[[nodiscard]] Iterator find(T const& value)
|
[[nodiscard]] Iterator find(T const& value)
|
||||||
{
|
{
|
||||||
return find(TraitsForT::hash(value), [&](auto& other) { return TraitsForT::equals(value, other); });
|
return find(TraitsForT::hash(value), [&](auto& entry) { return TraitsForT::equals(entry, value); });
|
||||||
}
|
}
|
||||||
|
|
||||||
template<typename TUnaryPredicate>
|
template<typename TUnaryPredicate>
|
||||||
|
@ -328,14 +328,14 @@ public:
|
||||||
|
|
||||||
[[nodiscard]] ConstIterator find(T const& value) const
|
[[nodiscard]] ConstIterator find(T const& value) const
|
||||||
{
|
{
|
||||||
return find(TraitsForT::hash(value), [&](auto& other) { return TraitsForT::equals(value, other); });
|
return find(TraitsForT::hash(value), [&](auto& entry) { return TraitsForT::equals(entry, value); });
|
||||||
}
|
}
|
||||||
// FIXME: Support for predicates, while guaranteeing that the predicate call
|
// FIXME: Support for predicates, while guaranteeing that the predicate call
|
||||||
// does not call a non trivial constructor each time invoked
|
// does not call a non trivial constructor each time invoked
|
||||||
template<Concepts::HashCompatible<T> K>
|
template<Concepts::HashCompatible<T> K>
|
||||||
requires(IsSame<TraitsForT, Traits<T>>) [[nodiscard]] Iterator find(K const& value)
|
requires(IsSame<TraitsForT, Traits<T>>) [[nodiscard]] Iterator find(K const& value)
|
||||||
{
|
{
|
||||||
return find(Traits<K>::hash(value), [&](auto& other) { return Traits<T>::equals(other, value); });
|
return find(Traits<K>::hash(value), [&](auto& entry) { return Traits<T>::equals(entry, value); });
|
||||||
}
|
}
|
||||||
|
|
||||||
template<Concepts::HashCompatible<T> K, typename TUnaryPredicate>
|
template<Concepts::HashCompatible<T> K, typename TUnaryPredicate>
|
||||||
|
@ -347,7 +347,7 @@ public:
|
||||||
template<Concepts::HashCompatible<T> K>
|
template<Concepts::HashCompatible<T> K>
|
||||||
requires(IsSame<TraitsForT, Traits<T>>) [[nodiscard]] ConstIterator find(K const& value) const
|
requires(IsSame<TraitsForT, Traits<T>>) [[nodiscard]] ConstIterator find(K const& value) const
|
||||||
{
|
{
|
||||||
return find(Traits<K>::hash(value), [&](auto& other) { return Traits<T>::equals(other, value); });
|
return find(Traits<K>::hash(value), [&](auto& entry) { return Traits<T>::equals(entry, value); });
|
||||||
}
|
}
|
||||||
|
|
||||||
template<Concepts::HashCompatible<T> K, typename TUnaryPredicate>
|
template<Concepts::HashCompatible<T> K, typename TUnaryPredicate>
|
||||||
|
|
|
@ -224,12 +224,12 @@ public:
|
||||||
|
|
||||||
ConstIterator find(T const& value) const
|
ConstIterator find(T const& value) const
|
||||||
{
|
{
|
||||||
return find_if([&](auto& other) { return Traits<T>::equals(value, other); });
|
return find_if([&](auto& entry) { return Traits<T>::equals(entry, value); });
|
||||||
}
|
}
|
||||||
|
|
||||||
Iterator find(T const& value)
|
Iterator find(T const& value)
|
||||||
{
|
{
|
||||||
return find_if([&](auto& other) { return Traits<T>::equals(value, other); });
|
return find_if([&](auto& entry) { return Traits<T>::equals(entry, value); });
|
||||||
}
|
}
|
||||||
|
|
||||||
template<typename U = T>
|
template<typename U = T>
|
||||||
|
|
|
@ -22,7 +22,7 @@ struct GenericTraits {
|
||||||
static constexpr bool is_trivially_serializable() { return false; }
|
static constexpr bool is_trivially_serializable() { return false; }
|
||||||
static constexpr bool equals(T const& a, T const& b) { return a == b; }
|
static constexpr bool equals(T const& a, T const& b) { return a == b; }
|
||||||
template<Concepts::HashCompatible<T> U>
|
template<Concepts::HashCompatible<T> U>
|
||||||
static bool equals(U const& a, T const& b) { return a == b; }
|
static bool equals(T const& self, U const& other) { return self == other; }
|
||||||
};
|
};
|
||||||
|
|
||||||
template<typename T>
|
template<typename T>
|
||||||
|
|
|
@ -91,7 +91,7 @@ struct Traits<NonnullOwnPtr<Kernel::KString>> : public GenericTraits<NonnullOwnP
|
||||||
using ConstPeekType = Kernel::KString const*;
|
using ConstPeekType = Kernel::KString const*;
|
||||||
static unsigned hash(NonnullOwnPtr<Kernel::KString> const& p) { return string_hash(p->characters(), p->length()); }
|
static unsigned hash(NonnullOwnPtr<Kernel::KString> const& p) { return string_hash(p->characters(), p->length()); }
|
||||||
static bool equals(NonnullOwnPtr<Kernel::KString> const& a, NonnullOwnPtr<Kernel::KString> const& b) { return a->view() == b->view(); }
|
static bool equals(NonnullOwnPtr<Kernel::KString> const& a, NonnullOwnPtr<Kernel::KString> const& b) { return a->view() == b->view(); }
|
||||||
static bool equals(StringView a, NonnullOwnPtr<Kernel::KString> const& b) { return a == b->view(); }
|
static bool equals(NonnullOwnPtr<Kernel::KString> const& a, StringView b) { return a->view() == b; }
|
||||||
};
|
};
|
||||||
|
|
||||||
template<>
|
template<>
|
||||||
|
@ -113,11 +113,11 @@ struct Traits<OwnPtr<Kernel::KString>> : public GenericTraits<OwnPtr<Kernel::KSt
|
||||||
|
|
||||||
return a->view() == b->view();
|
return a->view() == b->view();
|
||||||
}
|
}
|
||||||
static bool equals(StringView a, OwnPtr<Kernel::KString> const& b)
|
static bool equals(OwnPtr<Kernel::KString> const& a, StringView b)
|
||||||
{
|
{
|
||||||
if (!b)
|
if (!a)
|
||||||
return a.is_null();
|
return b.is_null();
|
||||||
return a == b->view();
|
return a->view() == b;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
@ -9,6 +9,7 @@
|
||||||
#include <AK/DeprecatedString.h>
|
#include <AK/DeprecatedString.h>
|
||||||
#include <AK/OwnPtr.h>
|
#include <AK/OwnPtr.h>
|
||||||
#include <AK/ReverseIterator.h>
|
#include <AK/ReverseIterator.h>
|
||||||
|
#include <AK/String.h>
|
||||||
#include <AK/Vector.h>
|
#include <AK/Vector.h>
|
||||||
|
|
||||||
TEST_CASE(construct)
|
TEST_CASE(construct)
|
||||||
|
@ -431,6 +432,13 @@ TEST_CASE(should_find_predicate_index)
|
||||||
EXPECT(!v.find_first_index_if([](auto const v) { return v == 123; }).has_value());
|
EXPECT(!v.find_first_index_if([](auto const v) { return v == 123; }).has_value());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST_CASE(should_find_using_a_hashcompatible_value)
|
||||||
|
{
|
||||||
|
// Tests whether a hash-compatible value can be used to compare (Strings cannot be impliticly constructed from a StringView.)
|
||||||
|
Vector v { "hello!"_string };
|
||||||
|
EXPECT(v.contains_slow("hello!"sv));
|
||||||
|
}
|
||||||
|
|
||||||
TEST_CASE(should_contain_start)
|
TEST_CASE(should_contain_start)
|
||||||
{
|
{
|
||||||
// Tests whether value is found if at the start of the range.
|
// Tests whether value is found if at the start of the range.
|
||||||
|
|
Loading…
Reference in a new issue