Re: Corrected style of comments (issue 5862052)

[k-ohara5a5a]

This patch adds helpful comments.


http://codereview.appspot.com/5862052/diff/1/flower/include/direction.hh
File flower/include/direction.hh (right):

http://codereview.appspot.com/5862052/diff/1/flower/include/direction.hh#newcode77
flower/include/direction.hh:77: * Thanks to a #define below, instead of
writing:
> Only you and Han Wenn have answered. Maybe other agree on
> changing do to for_UP_and_DOWN? How do I know?

You cannot know every individual opinion, so you must decide what is
wise.   You know that I fear that I will forget if there is any
difference between do{}while(flip()) and for_UP_and_DOWN.  You know that
HanWenn suggests changing all do{}while(flip) at once.

If you change just a few do{}while(flip) to your new idiom, then I will
be tempted to create a third idiom:
 for (Direction d = UP; d >= DOWN; d = (Direction)(d + DOWN - UP)

http://codereview.appspot.com/5862052/diff/1/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/5862052/diff/1/lily/note-collision.cc#newcode183
lily/note-collision.cc:183: */
Move the block comment above down below your new comments, to keep it
adjacent to the if...elseif... block that it describes.

http://codereview.appspot.com/5862052/diff/1/lily/note-collision.cc#newcode286
lily/note-collision.cc:286: // The offset should depend on line
thickness, not staff space, at least in some cases (like stem-to-stem,
where it should be bigger for smaller font size)
What does "the offset" refer to?  shift_amount?

http://codereview.appspot.com/5862052/diff/1/lily/note-collision.cc#newcode383
lily/note-collision.cc:383: void update_offsets (Drul_array<vector<Real>
> *offsets,
> What I was taught on the university is to write short
>   and simple functions that do only one thing.

In this case, the function is too short, relative to the complexity of
the interface(*), for separation.

(*) The function changes what is pointed to by its first argument, which
is not unusual, but milimetre88 found the same operation in another
function worth a comment with a '!'.
The second argument is used only for its dimensions, which are related
to the dimensions of the first argument for reasons we can understand
only by looking at the calling function.  I would be confused wondering
why the function does not simply use the dimensions of offsets[][].

http://codereview.appspot.com/5862052/