D8619: Refactor and remove duplicate code in kfileplacesview

David Faure noreply at phabricator.kde.org
Sun Nov 5 20:51:40 UTC 2017


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

INLINE COMMENTS

> kfileplacesview.cpp:66
> +    QSize sizeHint(const QStyleOptionViewItem &option,
>                             const QModelIndex &index) const Q_DECL_OVERRIDE;
> +    void paint(QPainter *painter,

re-indent after the removal of "virtual"

> kfileplacesview.cpp:68
> +    void paint(QPainter *painter,
>                         const QStyleOptionViewItem &option,
>                         const QModelIndex &index) const Q_DECL_OVERRIDE;

same here, please re-indent after the removal of "virtual"

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

You use qobject_cast elsewhere, why not here too?

Or even... you use static_cast for the delegate in 90% of the code, why two uses of qobject_cast?

If it's mandatory for the delegate to be a KFilePlacesViewDelegate, it's mandatory everywhere, right?

(too bad setItemDelegate isn't virtual, we could have caught it there...)

REPOSITORY
  R241 KIO

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

To: mlaurent, #frameworks, ervin, dfaure
Cc: dfaure, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171105/e7a4700d/attachment.html>


More information about the Kde-frameworks-devel mailing list