Comparing KFileItems

[Frank Reininghaus]

Hi,

2012/10/19 Michael Pyne:
> On Thursday, October 18, 2012 13:54:09 Frank Reininghaus wrote:
>> There are ways to fix or work around this problem inside Dolphin, but
>> I'm wondering if it makes sense to change the way KFileItems are
>> compared. If two items do not have the same d pointers, one could
>> check if their URLs are equal and consider them equal in that case.
>> Or, if that is not considered suffient for equality, do what
>> KFileItem::cmp(const KFileItem&) does.
>>
>> I see the following advantages and disadvantages:
>>
>> Pro:
>>
>> a) The implicit sharing is an implementation detail of KFileItem. At
>> least I would not have expected that operator==() depends on this and
>> only compares the d pointers.
>> b) It could possibly prevent other bugs in the future and save debugging
>> time.
>>
>> Con:
>>
>> Changing the way KFileItems are compared could considerably slow down
>> comparisons of items which are not equal.
>>
>> Any opinions about this?
>
> Well, your pros are both correct (and you didn't even list that the current
> behavior is inconsistent between KFileItem::cmp() and KFileItem::operator==()
> ;). But also, you list high CPU usage as a possible con, even though it's what
> got you into troubleshooting this in the first place!

Well, the high CPU usage we get in Dolphin is sort of an accident -
there are ways to work around this even if KFileItem::operator==() is
not changed. On the other hand, changing the behaviour will always
slow down comparisons of KFileItems that are really not equal. I don't
know if this would be a big concern, but I can't see any other good
reason why the current implementation is as it is.

> I would recommend comparing the d-ptrs as done now as an early optimization
> and then falling back to KFileItem::cmp() (or the inlined equivalent) if the
> d-ptrs are different.

That sounds very reasonable to me. Does anyone whith more insight into
common uses of KFileItem (David maybe?) have any objections?

Best regards,
Frank