D23268: add code documentation and minimal code smells improvement
Frederik Schwarzer
noreply at phabricator.kde.org
Mon Aug 19 18:43:00 BST 2019
schwarzer added a comment.
Thanks indeed. Just a note for future. Try to keep patches smaller. Separate them in e.g. whitespace changes, documentation changes and code changes. `git add -p` or `git commit -p` helps with that.
Reason: If you mix code changes with whitespace changes and then the code change turns out to be problematic, you cannot just revert the change without losing the whitespace fixes.
INLINE COMMENTS
> ball.cpp:111-114
> if ( m_framesNum > 1 )
> - {
> frame = KRandom::random() % m_framesNum;
> - }
This is valid C++ and some style guides allow leaving out the braces ... however, during my day job I found cases, where the code was altered later and the developer added one line and forgot about adding braces. So I would always use braces on an if statement. ... But this is just my opinion.
> gamewidget.cpp:39-48
> KBounceGameWidget::KBounceGameWidget( QWidget* parent )
> : QGraphicsView( parent )
> - , m_state( BeforeFirstGame )
> - , m_bonus( 0 )
> - , m_level( 0 )
> - , m_lives( 0 )
> - , m_time( 0 )
You kept the spaces inside the braces here. On purpose?
REPOSITORY
R392 KBounce
REVISION DETAIL
https://phabricator.kde.org/D23268
To: pedrohenriques, #kde_games
Cc: schwarzer, yurchor, kde-games-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20190819/1bd65710/attachment-0001.html>
More information about the kde-games-devel
mailing list