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