D13211: Enable comparing KFileItems by url

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfileitem.cpp:1241
> +{
> +    if (!d || !other.d) {
> +        return false;

This isn't symmetric. operator< must have the property that a<b and b<a aren't both true, and that they are both false only if the items are equal.

If `this` is a valid item and `other` is null, then this code says that *this<other is false and other<*this is false.
Which would let some algorithms deduce that *this == other, which is completely wrong.

If we decide that a null item is inferior to anything else, then we need

  if (!d) return true; if (!other.d) return false;

(with newlines and braces of course).

This also calls for a corresponding unittest.

> kfileitem.h:493
> +     */
> +    bool operator<(const KFileItem &other) const;
> +

This one looks good.

> kfileitem.h:499
> +     */
> +    bool operator<(const QUrl &other) const;
> +

This operator seems strange to me, it's comparing a KFileItem and a QUrl, which are two different kinds of objects... What's the use case?

REPOSITORY
  R241 KIO

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

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