dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed.
A good example of how a unittest helps catching a bug :-) (and a good example of how code that I suggest isn't necessarily bugfree, haha) Missing: unittests for the < QUrl overload. The case of the invalid item vs an invalid URL will be interesting too ;) INLINE COMMENTS > kfileitemtest.cpp:305 > + // a KFileItem without url is considered < than anything. > + QVERIFY(nulFileItem < nulFileItem); > + QVERIFY(nulFileItem < fileItem); I wonder if that should be true, though ;-) I guess it should rather be false, so that a binary search (based on < only) can deduce that a null item is *equal* to another null item, rather than both being lower than the other, which would be nonsense. > kfileitemtest.cpp:314 > + QVERIFY(!(fileItem3 < fileItem)); > + // This should be false as they are equal > + QVERIFY(!(fileItem < fileItem)); ... exactly. I think the same reasoning applies to a null item. > kfileitem.h:490 > + /** > + * Returns true if other's URL is lexically greater than this url; > otherwise returns false > + * @since 5.47 "this item's URL" > kfileitem.h:496 > + /** > + * Returns true if url other is lexically greater than this url; > otherwise returns false > + * @since 5.47 "than this item's URL", then REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D13211 To: jtamate, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
