D12659: two new UDS structures

David Faure noreply at phabricator.kde.org
Thu May 3 08:06:57 UTC 2018


dfaure added a comment.


  I like the replaceOrInsert idea, and the assert in insert... we might have to fix some kioslaves, but in general they have no good reason to insert twice for the same key.
  
  In general I like the std::vector + find_if + emplace_back solution. Fast and readable.

INLINE COMMENTS

> udsentry_benchmark.cpp:500
> +    {
> +        Q_ASSERT(std::find_if(storage.begin(), storage.end(),
> +                                  [field](const Field &index) {return index.m_index == field;}) == storage.end());

storage.constBegin(), storage.constEnd() -- same in next method

> udsentry_benchmark.cpp:501
> +        Q_ASSERT(std::find_if(storage.begin(), storage.end(),
> +                                  [field](const Field &index) {return index.m_index == field;}) == storage.end());
> +        storage.push_back(Field(field, value));

constEnd() here too

> udsentry_benchmark.cpp:502
> +                                  [field](const Field &index) {return index.m_index == field;}) == storage.end());
> +        storage.push_back(Field(field, value));
> +    }

storage.emplace_back(field, value);

> udsentry_benchmark.cpp:513
> +        }
> +        storage.push_back(Field(field, value));
> +    }

emplace_back

> udsentry_benchmark.cpp:538
> +    {
> +        auto index = std::find_if(storage.begin(), storage.end(),
> +                                  [field](const Field &index) {return index.m_index == field;});

call it "it", find_if returns an iterator.

> udsentry_benchmark.cpp:601
> +    {
> +        auto index = std::lower_bound(storage.begin(), storage.end(), field, less);
> +        Q_ASSERT(index == storage.end() || index->m_index != field);

constBegin() etc. to avoid detaching
it = ...

> udsentry_benchmark.cpp:625
> +        if (index != storage.end() && index->m_index == field ) {
> +            Q_ASSERT(index->m_str != QStringLiteral());
> +            index->m_long = value;

!index->m_str.isEmpty()

QStringLiteral() without arguments never makes sense, there's no literal...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12659

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180503/9e914a09/attachment.html>


More information about the Kde-frameworks-devel mailing list