Review Request 129099: Make m_drawingEngine a std::unique_ptr
Oliver Sander
oliver.sander at tu-dresden.de
Wed Oct 5 08:56:05 UTC 2016
> On Oct. 4, 2016, 9 p.m., Albert Astals Cid wrote:
> > I honestly don't see the benefit
> >
> > You changed a delete by a reset, is there any benefit at all other than making the code harder to read?
>
> Oliver Sander wrote:
> I see three benefits:
> a) I can see by looking at the declaration of m_drawingEngine that it has ownership semantics. IMO that's quite helpful when trying to understand new code.
> b) I don't have to call 'delete' (admittedly that's minor here, because m_drawingEngine is deleted only once).
> c) Have a look at https://git.reviewboard.kde.org/r/128858 . There I had to introduce
>
> delete m_drawingEngine;
> m_drawingEngine = new SmoothPathEngine( m_currentDrawingToolElement );
>
> With a unique_ptr this is simply
>
> m_drawingEngine = std::make_unique<SmoothPathEngine>( m_currentDrawingToolElement );
Alternatively, the last line could be
m_drawingEngine.reset( new SmoothPathEngine( m_currentDrawingToolElement ) );
which makes the intention even clearer.
- Oliver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129099/#review99787
-----------------------------------------------------------
On Oct. 4, 2016, 8:33 p.m., Oliver Sander wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129099/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2016, 8:33 p.m.)
>
>
> Review request for Okular.
>
>
> Repository: okular
>
>
> Description
> -------
>
> Because it has ownership semantics.
>
> The code would be even shorter with std::make_unique, but that is C++14. Is that allowed in Okular?
>
>
> Diffs
> -----
>
> ui/presentationwidget.h 69574d2
> ui/presentationwidget.cpp c16d616
>
> Diff: https://git.reviewboard.kde.org/r/129099/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Oliver Sander
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20161005/5381e8c4/attachment.html>
More information about the Okular-devel
mailing list