Re: Corrected comments and a function check_meshing_chords divided in two. (issue 5975054)

[graham]

could you split this into 2 (or more) separate patches?

- some of your comments are good, and could have been pushed a month
ago.
- the macro changes are highly problematic and probably won't be
accepted for at least a few months.

I never like seeing good changes postponed due to them being squashed
together with questionable ones.


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

http://codereview.appspot.com/5975054/diff/1/lily/accidental-placement.cc#newcode116
lily/accidental-placement.cc:116: //TODO: add comment to this struct
this still doesn't add any useful information.  I mean, of course it
would be nice to explain stuff in the code.  But there's no point going
around adding
// TODO: comment this
to every single struct/class/method/function.

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

http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc#newcode404
lily/note-collision.cc:404: for_UP_and_DOWN (d)
On 2012/03/31 21:04:35, Keith wrote:
> If you write the 'for' loop directly in the code, then neither humans
nor
> auto-indent programs need to learn a macro:
> for (Direction d = UP; d; d = (UP == d)? DOWN : CENTER)

Ouch.  I don't find that for loop to be particularly easy to understand;
it would be much nicer if there was a macro for this.

I wonder if we could ask astyle to add a
--custom-loop-commands="for_UP_and_DOWN" option?  it would take a while
for that to reach an official release, but at least there would be some
hope of simplifying this.  I agree that we shouldn't switch to a macro
if that ruins the indentation.

http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc#newcode577
lily/note-collision.cc:577: for_UP_and_DOWN (d) // please, make a
comment to this loop (better than the above one...)
adding a comment to say "please comment this" does not help

http://codereview.appspot.com/5975054/