Re: some comments and complaints on the code (issue 5651069)

[milimetr88]

http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc
File lily/accidental-placement.cc (right):

http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc#newcode211
lily/accidental-placement.cc:211: * @return A vector of
Accidental_placement_entrys
Do you mean @param and @return or the comment to the function? What
comment would you propose?

On 2012/02/11 00:09:00, joeneeman wrote:
> Please don't just comment on the type (unless it's SCM in which case
you can say
> which scheme type it is).

http://codereview.appspot.com/5651069/diff/1/lily/include/note-column.hh
File lily/include/note-column.hh (right):

http://codereview.appspot.com/5651069/diff/1/lily/include/note-column.hh#newcode39
lily/include/note-column.hh:39: /**
Ok, I'll correct that.

On 2012/02/10 23:49:24, Carl wrote:
> IMO, this comment belongs in the definition, not the declaration

http://codereview.appspot.com/5651069/diff/1/lily/include/note-column.hh#newcode39
lily/include/note-column.hh:39: /**
On 2012/02/10 23:49:24, Carl wrote:
> IMO, this comment belongs in the definition, not the declaration

Done.

http://codereview.appspot.com/5651069/diff/1/lily/include/staff-symbol-referencer.hh
File lily/include/staff-symbol-referencer.hh (right):

http://codereview.appspot.com/5651069/diff/1/lily/include/staff-symbol-referencer.hh#newcode53
lily/include/staff-symbol-referencer.hh:53: /**
Ok, I'll correct that.

On 2012/02/10 23:49:24, Carl wrote:
> Again, comment in the definition, not the declaration

http://codereview.appspot.com/5651069/diff/1/lily/include/staff-symbol-referencer.hh#newcode53
lily/include/staff-symbol-referencer.hh:53: /**
On 2012/02/10 23:49:24, Carl wrote:
> Again, comment in the definition, not the declaration

Done.

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

http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc#newcode377
lily/note-collision.cc:377: /**
AFAIK, not really. In Contributor's Guide there are only 2 short
paragraphs:

Comments may not be needed if descriptive variable names are used in the
code and the logic is straightforward. However, if the logic is
difficult to follow, and particularly if non-obvious code has been
included to resolve a bug, a comment describing the logic and/or the
need for the non-obvious code should be included.

There are instances where the current code could be commented better. If
significant time is required to understand the code as part of preparing
a patch, it would be wise to add comments reflecting your understanding
to make future work easier.

(http://lilypond.org/doc/v2.15/Documentation/contributor-big-page#code-comments)

I just wanted to introduce JavaDoc comment style for autodocumenting.
Maybe style of comments should be discussed on lilypond-devel?

On 2012/02/10 23:49:24, Carl wrote:
> I don't like the double * following the /.  Is this a standard
anywhere else in
> lilypond?

http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc#newcode552
lily/note-collision.cc:552: for_UP_and_DOWN (d) // please, make a
comment to this loop (better than the above one...)
On 2012/02/10 23:49:24, Carl wrote:
> To whom is the "please" directed?  Is there anybody who is better than
you right
> now to comment the loop?

Yes - someone who knows this code :] Possibly one day someone will try
to refactor this code, what will mean that he understands it well.
For me it's really hard to understand whats going on here. Me and Janek
recently spent a couple of hours trying to understand several files
including this one and we still don't know if clash_groups[d] is one
single note or a group of notes.

Do you have a tip, how to work it out? Unfortunately it's not state in
the code.

http://codereview.appspot.com/5651069/