D14931: Eliminate duplicate QMaps in OutputWidget

Pino Toscano noreply at phabricator.kde.org
Sun Aug 19 21:19:37 BST 2018


pino added inline comments.

INLINE COMMENTS

> outputwidget.cpp:278-279
>              }
> -            delete view;
> -        } else
> -        {
> -            m_views.value( id )->setModel( nullptr );
> -            m_views.value( id )->setItemDelegate( nullptr );
> -            if( m_proxyModels.contains( 0 ) ) {
> -                delete m_proxyModels.take( 0 );
> -                m_filters.remove( 0 );
> +            m_views[id].destroy();
> +            m_views.remove(id);
> +        } else {  // KDevelop::IOutputView::OneView

This can be simplified in one single lookup:

  auto view = m_views.take(id);
  view.destroy();

> outputwidget.cpp:292-293
>          }
> +        m_views[id].destroy();
>          m_views.remove( id );
>          emit outputRemoved( data->toolViewId, id );

Ditto.

> outputwidget.cpp:704-711
> +    if (view) {
> +        delete view;
> +        view = nullptr;
> +    }
> +    if (proxyModel) {
> +        delete proxyModel;
> +        proxyModel = nullptr;

`delete` is a no-op on null pointers, so you can simply do:

  delete foo;
  foo = nullptr;

Of course, assuming you do not need to use the pointer just before its deletion.

> outputwidget.h:110
> +    };
> +    QMap<int, FilteredView>::iterator getFilteredView(QAbstractItemView *view);
> +

I'd personally name it `findFilteredView()`, and make it `const`, and return a `const_iterator`. At least from a quick glance, it seems like the iterator returned is always not changed/removed.

> outputwidget.h:112
> +
> +    QMap<int, FilteredView> m_views;
>      QTabWidget* m_tabwidget;

Unless you need the values sorted by the key (which is QMap does), I suggest to switch also to QHash.

REPOSITORY
  R32 KDevelop

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

To: vkorneev
Cc: pino, kdevelop-devel, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180819/4e3b91e4/attachment.html>


More information about the KDevelop-devel mailing list