Review Request 108386: Reorganise Dolphin's sorting code

[Emmanuel Pescosta]

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

Ship it!


Works without problems and very nice code btw. ;)

- Emmanuel Pescosta


On Jan. 13, 2013, 12:50 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108386/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2013, 12:50 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> At the moment, the sorting code is more complicated than it needs to be:
> 
> 1. Some functionality that is provided by Qt and/or the STL is duplicated. It seems that it had been overlooked that all Qt/STL functions that require a 'lessThan' comparison function can in fact be used even if the comparison function is a member of a class with the trick
> 
> class KFileItemModelLessThan
> {
> public:
>     KFileItemModelLessThan(const KFileItemModel* model) :
>         m_model(model)
>     {
>     }
> 
>     bool operator()(const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b) const
>     {
>         return m_model->lessThan(a, b);
>     }
> 
> private:
>     const KFileItemModel* m_model;
> };
> 
> and then:
> 
> KFileItemModelLessThan lessThan(model);
> 
> 2. KFileItemModel-specific code and the parallel sorting implementation are mixed in the same file.
> 
> With this commit [1], I try to resolve these issues. People who need parallel sorting in other contexts where comparisons are expensive and reentrant can now just use the templates in kfileitemmodelsortalgorithm.h. Moreover, it's very easy now to replace the sort function by std::sort, std::stable_sort and others for benchmarking and other experiments.
> 
> Does anyone see a problem with this approach or a bug that I might have introduced accidentally? If not, I'll push that to master in a few days.
> 
> [1] In fact, I did it in three smaller steps:
> 
> http://paste.kde.org/645824/
> http://paste.kde.org/645830/
> http://paste.kde.org/645836/
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 4b31ab6 
>   dolphin/src/kitemviews/kfileitemmodel.h ef9dc98 
>   dolphin/src/kitemviews/kfileitemmodel.cpp f46182b 
>   dolphin/src/kitemviews/private/kfileitemmodelsortalgorithm.h 07e5d4a 
>   dolphin/src/kitemviews/private/kfileitemmodelsortalgorithm.cpp ab650ef 
> 
> Diff: http://git.reviewboard.kde.org/r/108386/diff/
> 
> 
> Testing
> -------
> 
> Sorting still works OK.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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