Review Request: Add Category and NewGame classes

[Shantanu Tushar Jha]

> On July 1, 2011, 3:43 p.m., Arjen Hiemstra wrote:
> > Category's function seems relatively logical. However, to me, NewGame sounds more like a method on some other class. I understand that it is async so you need to implement the signal handlers, but it seems to me that it is better suited for a more generic GameManager or something, that can also handle existing games and such, which would then include a newGame() method.

Ok, though I think its much more cleaner to separate different actions into different classes (not generally, only in this case. Here, the classes are more like jobs (so one object per request), so if you combine two classes, you create a GameManager object even when what you required was just what NewGame provided, this wasting mem as some private vars aren't used in NewGame but only in EditGame. You will only set gameName and gameCategory, all other vars are useless).
Stil, I've attached an updated diff to show what it looks like.


> On July 1, 2011, 3:43 p.m., Arjen Hiemstra wrote:
> > player/lib/category.h, line 58
> > <http://git.reviewboard.kde.org/r/101816/diff/1/?file=25763#file25763line58>
> >
> >     Personally, I prefer the following order in general:
> >     
> >     Public
> >     Public Slots
> >     Signals
> >     Protected
> >     Protected Slots
> >     Private
> >     Private Slots
> >     
> >     Makes it easier to simply read the header to find out what methods it contains and makes it easier to skip over the irrelevant ones. (i.e., private always and protected when you're not inheriting.)
> >     
> >     Also, wouldn't these be better served by using Q_PRIVATE_SLOT ?

I have read concerns about Q_PRIVATE_SLOT not being documented and hence they should be avoided. (Refer http://lists.trolltech.com/qt-interest/2008-04/msg00348.html, first comment from Thiago). Is it ok to be used?


- Shantanu Tushar


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


On July 1, 2011, 2:40 p.m., Shantanu Tushar Jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101816/
> -----------------------------------------------------------
> 
> (Updated July 1, 2011, 2:40 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Category's job is to fetch list of categories from the server.
> NewGame takes a game name and a category, and then creates a new game on the server, returning the ID of newly created game.
> 
> 
> Diffs
> -----
> 
>   player/lib/CMakeLists.txt 53c0f0a 
>   player/lib/category.h PRE-CREATION 
>   player/lib/category.cpp PRE-CREATION 
>   player/lib/newgame.h PRE-CREATION 
>   player/lib/newgame.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101816/diff
> 
> 
> Testing
> -------
> 
> Both classes function as expected when used.
> 
> 
> Thanks,
> 
> Shantanu Tushar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/gluon/attachments/20110702/04c7866a/attachment.htm