[rekonq] Re: Review Request: A bit of cleanup in Application.

Pierre Rossi pierre.rossi at gmail.com
Sun Jan 9 00:18:30 CET 2011



> On Jan. 8, 2011, 10:13 p.m., Benjamin Poulain wrote:
> > You are gonna kill me. The previous patch was more correct actually :)
> > 
> > I did not notice those objects are parented to the instance of Application. One have to wonder why those methods are static at all.

Huh indeed, that might actually crash on exit now! 
The static methods still seem more convenient than something like Application::instance()->historyManger() or similar calls...
Hard to decide which approach to keep for cleanup, I'm tempted to drop the instance() as a parent in favor of a scoped pointer because it's more generic, so if some day someone wants to add something that's not a QObject in the same fashion, it can still be done in the same way.


- Pierre


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


On Jan. 8, 2011, 9:51 p.m., Pierre Rossi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100316/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2011, 9:51 p.m.)
> 
> 
> Review request for rekonq and Benjamin Poulain.
> 
> 
> Summary
> -------
> 
> I believe we don't need static members in QWeakPointers for all the *Managers, static getter functions would do the job.
> 
> 
> This addresses bug N/A.
>     /show_bug.cgi?id=N/A
> 
> 
> Diffs
> -----
> 
>   src/application.h b30e337 
>   src/application.cpp 466a0a4 
> 
> Diff: http://git.reviewboard.kde.org/r/100316/diff
> 
> 
> Testing
> -------
> 
> "compiles and runs" ™
> 
> 
> Thanks,
> 
> Pierre
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20110108/9ca32eb1/attachment.htm 


More information about the rekonq mailing list