[Kde-pim] Review Request 111101: Use QWeakPointer to store move|copy and selectfolder dialogs

Milian Wolff mail at milianw.de
Tue Jun 18 21:13:36 BST 2013



> On June 18, 2013, 5:36 p.m., Christian Mollekopf wrote:
> > I agree that you should be using QPointer instead of QWeakPointer. QWeakPointer models a weak reference while your pointer holds ownership, which is quite confusing.
> 
> Aurélien Gâteau wrote:
>     I do not understand QPointer this way. QWeakPointer can be used as a replacement for QPointer, as the doc said. Both should be used to manage a pointer whose data is owned by another object. In the current situation, the owner of the dialogs is the KMMainWidget since it is used as the dialog parent.

This is true, neither QPointer or QWeakPointer keep ownership.

You can use QPointer though since 4.6 it is implemented using QWeakPointer internally. Thus it is just as efficient but not as cumbersome to use. See also: http://www.macieira.org/blog/2012/07/continue-using-qpointer/


- Milian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111101/#review34609
-----------------------------------------------------------


On June 18, 2013, 7:30 p.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111101/
> -----------------------------------------------------------
> 
> (Updated June 18, 2013, 7:30 p.m.)
> 
> 
> Review request for KDEPIM and David Faure.
> 
> 
> Description
> -------
> 
> The code showing the move|copy and select folder dialogs tries to check the dialogs are not deleted while they are visible, but it actually recreates them instead of checking their existence. The patch changes this to use QWeakPointer to track the dialogs instead.
> 
> 
> Diffs
> -----
> 
>   kmail/kmmainwidget.h 977e1e13a85d703f71c234697d5cf7fe72577843 
>   kmail/kmmainwidget.cpp 36e7164a58d59cd35f296b9ab6885f1de8c44572 
> 
> Diff: http://git.reviewboard.kde.org/r/111101/diff/
> 
> 
> Testing
> -------
> 
> Dialogs still show and work correctly.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


More information about the kde-pim mailing list