[Kde-games-devel] Review Request: Porting Kapman from Phonon to KgSound

[Roney Gomes]

> On March 22, 2012, 11:48 a.m., Frederik Schwarzer wrote:
> > /trunk/KDE/kdegames/kapman/game.cpp, lines 37-44
> > <http://svn.reviewboard.kde.org/r/6914/diff/1/?file=47721#file47721line37>
> >
> >     Would it not better to put these into the initializer list? This way they are intialised twice, iirc.
> 
> Roney Gomes wrote:
>     Can you point another place in the code where the sounds are also loaded? I couldn't see it.
> 
> Frederik Schwarzer wrote:
>     I thought the right thing and said the wrong thing.
>     If you have elements of a class type (here KgSound), they are all initialised in stage 1 of the constructor. Stage 2 is everything between { and }. So in stage 1, the standard constructor of the element is called and the element is initialised and assigned some standard value. In stage 2 you then assign the real value. So two values are assigned.
>     By using an initialiser list, you can avoid the double assignment.
> 
> Roney Gomes wrote:
>     I see your point, but there's nothing wrong with the calls to KgSound's constructor. To base my point I'll paste here the constructor's first lines:
>     
>     KgSound::KgSound(const QString& file, QObject* parent)
>          : QObject(parent)
>          , d(new Private)
>     
>     As you can see, the data stored is not doubly initialized.

I have to apologize and withdraw my argument, I was wrong. In fact the objects were really being doubly initialized, thanks for pointing me that.

The new diff I have updated solves this issue.


- Roney


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6914/#review10728
-----------------------------------------------------------


On March 23, 2012, 2:20 p.m., Roney Gomes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6914/
> -----------------------------------------------------------
> 
> (Updated March 23, 2012, 2:20 p.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Description
> -------
> 
> I've ported kapman to KgSound as requested in the mailing list. The files changed were game.cpp and game.h.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/kapman/game.h 1286463 
>   /trunk/KDE/kdegames/kapman/game.cpp 1286463 
> 
> Diff: http://svn.reviewboard.kde.org/r/6914/diff/
> 
> 
> Testing
> -------
> 
> I've played and observed whether the sounds were been played in the proper situations.
> 
> 
> Thanks,
> 
> Roney Gomes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20120323/388cf3b2/attachment.html>