Review Request: Add Category and NewGame classes

[Arjen Hiemstra]

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


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.


player/lib/category.h
<http://git.reviewboard.kde.org/r/101816/#comment3517>

    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 ?


- Arjen


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/20110701/d58c23cc/attachment.htm