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