D16663: [import] Fix leaking AbstractFileFilters

Christoph Roick noreply at phabricator.kde.org
Wed Dec 5 23:16:39 GMT 2018


croick marked an inline comment as done.
croick added inline comments.

INLINE COMMENTS

> asemke wrote in LiveDataSource.cpp:252
> this needs only be done if m_filter != nulltpr,

Actually nothing happens when deleting a nullptr (https://en.cppreference.com/w/cpp/language/delete). The check would be redundant. For newer versions of C++ this assumption seems to change a little, in case a non-standard deallocator is used – which we don't, the standard deallocator just does nothing with a nullptr.

> asemke wrote in ImportFileWidget.cpp:595
> I think the following would also to the job completely without any smart pointers and static casts:
> 
>   if (m_currentFilter)
>       delete m_currentFilter;
>   
>   auto filter = new AsciiFilter();
>   filter->set*();
>   m_currentFilter = filter;

The point of keeping the filter is not just a correct cleanup, but also to omit reparsing the file that is to be imported. `currentFileFilter()` gets called a lot and makes the UI unresponsive otherwise. (Which was my initial reason for this patch.)
I could switch to a raw pointer still, but that would be inconsistent with the pointers for the options widgets.

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/20181205/90abecf6/attachment.html>


More information about the kde-edu mailing list