D8243: Implement support for categories on KfilesPlacesView

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfileplacesview.cpp:997
> +{
> +    KFilePlacesViewDelegate *delegate = dynamic_cast<KFilePlacesViewDelegate *>(itemDelegate());
> +

If you are sure that the delegate is a KFilePlacesViewDelegate, then use static_cast.
If you aren't sure, then test for nullptr.

An unchecked dynamic_cast makes no sense.

> kfileplacesview.cpp:1253
> +{
> +    QSet<QString> sections;
> +

Missing

  sections.reserve(q->model()->rowCount())

(well, then better put the rowCount into a local variable, anyway).

Overall, filling a set just to count occurences seems like much work for just one number in the end, but I don't know if there's a faster algorithm that can be used. Would I be correct if I said that those GroupRole values are sorted? Couting unique values in a list like {A, A, B, B, B, C, C} doesn't require a full QSet to be filled in, one can just increment the count when the current value differs from the previous one.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8243

To: renatoo, #frameworks, dfaure, ervin
Cc: mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171012/fd6777c7/attachment.html>