[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Corrected style of comments (issue 5862052)

From: k-ohara5a5a
Subject: Re: Corrected style of comments (issue 5862052)
Date: Fri, 23 Mar 2012 07:23:36 +0000

This patch adds helpful comments.
File flower/include/direction.hh (right):
flower/include/direction.hh:77: * Thanks to a #define below, instead of
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)
File lily/ (right):
lily/ */
Move the block comment above down below your new comments, to keep it
adjacent to the if...elseif... block that it describes.
lily/ // 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?
lily/ void update_offsets (Drul_array<vector<Real>
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[][].

reply via email to

[Prev in Thread] Current Thread [Next in Thread]