Review Request 108766: Simplify comparison of children of expanded folders

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108766/#review27134
-----------------------------------------------------------


This review has been submitted with commit b80384caae111e5c9145ada293922acafe5fa085 by Frank Reininghaus to branch master.

- Commit Hook


On Feb. 3, 2013, 11:10 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108766/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2013, 11:10 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> The comparison of items when expanded folders are present, which is done mainly in the function KFileItemModel::expandedParentsCountCompare() is unnecessarily complex. It analyzes the sub-folder structure based on the URLs of both items. This is quite complicated and can cause problems in some situations, see bug 311947.
> 
> I propose to remove most of the current code and just use the 'parent', which is stored in the ItemData for each item, to compare the items. This requires much less code, is faster, and less buggy. 
> 
> This patch consists of three parts:
> 
> 1. The changes in KFileItemModel (including additional checks in KFileItemModel::isConsistent()),
> 2. A new unit test for bug 311947,
> 3. A new benchmark for adding and removing many items in a tree structure.
> 
> Comments welcome!
> 
> 
> This addresses bug 311947.
>     http://bugs.kde.org/show_bug.cgi?id=311947
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.h 903291a 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 1062aa5 
>   dolphin/src/tests/kfileitemmodelbenchmark.cpp 7017863 
>   dolphin/src/tests/kfileitemmodeltest.cpp 2ad1842 
> 
> Diff: http://git.reviewboard.kde.org/r/108766/diff/
> 
> 
> Testing
> -------
> 
> I cannot reproduce the crash any more. Existing tests still work OK. The benchmark for adding and removing many items in a tree structure runs in about 1.7 seconds, rather than 2.9 seconds.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130210/dd47f490/attachment.htm>