D12659: two new UDS structures

Stefan BrĂ¼ns noreply at phabricator.kde.org
Wed May 2 21:32:29 UTC 2018


bruns added inline comments.

INLINE COMMENTS

> udsentry_benchmark.cpp:483
> +        inline Field(const uint index, long long value = 0) : m_long(value), m_index(index) {}
> +        // This operator is essential to gain some speed, because the default == is slow
> +        inline bool operator == (const Field &other) const {

This comment is still wrong - you want to compare the key only, not the whole entry

Also, you no longer need it, as you use a lambda for the comparision now.

> udsentry_benchmark.cpp:488
> +
> +        QString m_str = QStringLiteral();
> +        long long m_long = LLONG_MIN;

non-POD types should not be initialized explicitly

> udsentry_benchmark.cpp:490
> +        long long m_long = LLONG_MIN;
> +        uint m_index = -1;
> +    };

`unsigned int` and -1?

> udsentry_benchmark.cpp:498
> +    }
> +    void insert(uint field, const QString &value)
> +    {

bad naming, you use field and class Field for different things

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

bad naming again, index is not an index but an entry or field

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/20180502/a069dea5/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list