D16663: [import] Use QSharedPointer for file filters

Alexander Semke noreply at phabricator.kde.org
Wed Nov 28 21:32:37 GMT 2018


asemke requested changes to this revision.
asemke added a comment.
This revision now requires changes to proceed.


  @croick thanks for addressing this. The memory leaks ImportFileWidget::currentFilter() are known for long time already but we didn't manage to clean up here yet... Thanks for addressing this now!
  
  Couple of comments
  
  - QSharedPointer has additional overhead for reference counting which we don't need in this case. Can we use std::unique_ptr here?
  - The new function AbstractFileFilter::type() can be implemented in the base class only with
  
    FileType AbstractFileFilter::type() {
        return m_type;
    }
  
  where m_type is a protected member in AbstractFileFilter which is initialized in the constructors of the different file filter classes. With this we don't need implemented this function in every filter class and to touch all header files.
  
  - In LabPlot we very frequently use  raw pointers as function parameters since the ownership and the life cycle  of them is very clear. For objects that only read the content of such pointers there is no need to use smart pointers to transfer the ownership or to do the reference counting. updateContent() in the options widgets is also such a case where we don't need any ref-counting and where a raw pointer is totally fine.

REPOSITORY
  R262 LabPlot

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

To: croick, #labplot, asemke
Cc: asemke, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20181128/1d371dbb/attachment.html>


More information about the kde-edu mailing list