> 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 ); > > Oliver Sander wrote: > Alternatively, the last line could be > > m_drawingEngine.reset( new SmoothPathEngine( > m_currentDrawingToolElement ) ); > > which makes the intention even clearer.
The code doesn't certainly seems clearer to me, but probably i'm just old. I'll let others that are actually developing the frameworks branch decide. - Albert ----------------------------------------------------------- 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 > >
