Comparing KFileItems

[Frank Reininghaus]

Hi David,

I see that I should probably have created a review request to make
review easier for you - sorry about that! But I think we're getting
closer to the final solution, so I'll just reply to your message with
a new patch.

2012/10/23 David Faure:
> On Monday 22 October 2012 20:10:23 Frank Reininghaus wrote:

>>  bool KFileItem::operator!=(const KFileItem& other) const
>>  {
>> -    return d != other.d;
>> +    return d != other.d && d->m_url != other.d->m_url;
>>  }
>
> Or return !operator==(other), inline, for easier maintainance.

I haven't implemented the inline part yet. Would you prefer to have
the inline method inside the class definition or rather just move the
function into the header file and prepend it with 'inline'? Or just
leave it as it is right now in my patch?

>
>>  #ifndef KDE_NO_DEPRECATED
>> diff --git a/kio/tests/kfileitemtest.cpp b/kio/tests/kfileitemtest.cpp
>> index fa88af7..1ac5d98 100644
>> --- a/kio/tests/kfileitemtest.cpp
>> +++ b/kio/tests/kfileitemtest.cpp
>> @@ -108,11 +108,12 @@ void KFileItemTest::testDetach()
>>      fileItem2.mark();
>>      QVERIFY(fileItem2.isMarked());
>>      QVERIFY(!fileItem.isMarked());
>> -    QVERIFY(fileItem != fileItem2);
>> +    QVERIFY(fileItem == fileItem2);
>
> The point of that test was to ensure that detaching did happen.
> Maybe add "friend class KFileItemTest;" and compare the value of the d
> pointers?
> In addition to QVERIFY(fileItem == fileItem2), of course.

Done.

>>      fileItem = fileItem2;
>>      QVERIFY(fileItem2.isMarked());
>>      QVERIFY(fileItem == fileItem2);
>> +    QVERIFY(!(fileItem != fileItem2));
>>  }
>>
>>  void KFileItemTest::testBasic()
>> @@ -246,7 +247,8 @@ void KFileItemTest::testCmp()
>>
>>      KFileItem fileItem(KFileItem::Unknown, KFileItem::Unknown,
>> KUrl(file.fileName()), true /*on demand*/); KFileItem
>> fileItem2(KFileItem::Unknown, KFileItem::Unknown, KUrl(file.fileName()),
>> false); -    QVERIFY(fileItem != fileItem2); // created independently so
>> not 'equal' +    QVERIFY(fileItem == fileItem2); // created independently,
>> but still 'equal' +    QVERIFY(!(fileItem != fileItem2));
>>      QVERIFY(fileItem.cmp(fileItem2));
>
> Thank you kmail for messing up the lines in the reply...
> The change looks good though.
>
> Could you add a bit of docu while at it, about the difference between == and
> cmp as discussed here? Thanks.

Done.

Best regards,
Frank
-------------- next part --------------
A non-text attachment was scrubbed...
Name: KFileItem-version2.diff
Type: application/octet-stream
Size: 3060 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121024/620130ff/attachment.obj>