[webkit-reviews] review granted: [Bug 227843] [ITP] Improve ResourceLoadStatisticsDatabaseStore's buildList() and use it in more places : [Attachment 433225] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jul 9 17:15:53 PDT 2021
Sam Weinig <sam at webkit.org> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 227843: [ITP] Improve ResourceLoadStatisticsDatabaseStore's buildList() and
use it in more places
https://bugs.webkit.org/show_bug.cgi?id=227843
Attachment 433225: Patch
https://bugs.webkit.org/attachment.cgi?id=433225&action=review
--- Comment #3 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 433225
--> https://bugs.webkit.org/attachment.cgi?id=433225
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=433225&action=review
>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:330
> +static String buildList(const ContainerType& values)
One day I will add the following to WTF:
template <typename ContainerType, typename SeparatorType, typename
TransformFunctor> String join(const ContainerType& values, const SeparatorType&
separator, TransformFunctor&& functor)
{
StringBuilder builder;
bool first = true;
for (auto& value : values) {
auto tuple = std::invoke(functor, value);
if (first) {
builder.append(tuple);
first = false;
} else
builder.append(separator, tuple);
}
return builder.toString();
}
so this becomes
template <typename ContainerType> static String buildList(const ContainerType&
values)
{
return join(values, ", ", [](auto& value) {
if constexpr
(std::is_arithmetic_v<std::remove_reference_t<decltype(value)>>)
return std::make_tuple('"', value, '"');
else
return value;
});
}
I'm unclear it it is better :).
>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:338
> + if constexpr
(std::is_arithmetic_v<std::remove_reference_t<decltype(value)>>) {
> + builder.append(builder.isEmpty() ? "" : ", ", value);
> + } else {
> + builder.append(builder.isEmpty() ? "" : ", ", '"', value, '"');
> + }
No need for the braces here. I think it is likely a bit more efficient to use
dedicated bool rather than check isEmpty().
More information about the webkit-reviews
mailing list