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

[k-ohara5a5a]

Splitting the function in two doesn't make it any easier for me to
understand, but I had figured it out before.

On 2012/03/21 18:56:08, Milimetr88 wrote:
> What I was taught at the university is to write short
> and simple functions that do only one thing.

Maybe this was intended as advice for when you initially write code; it
would encourage the writer to find the smallest independent tasks and
cleanest interfaces between those tasks.  Splitting up an existing
function, that has grown into its assigned task and assigned interface,
is different.

I can find no better place to divide the function than where you have:
one function to characterize the meshing of note-heads and the rest to
determine how to shift the chords.

The interface is complicated because pass several properties of the two
chords.  Maybe in determine_collision_type() you could just look them up
again through the two Stems?


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#newcode45
lily/note-collision.cc:45: /* Filter out the 'o's in this configuration,
since they're no
On 2012/04/14 13:56:18, Milimetr88 wrote:
> indeed dps is not needed, *BUT* ups is - unfortunately

The use of ups[0] on line 230 is correct(*) whether or not suspended
notes are included.  Passing the variable between functions is not
required.
*-correct for all the cases LilyPond handles to date; David will
generalizing this code shortly.

http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc#newcode238
lily/note-collision.cc:238: /* The solfa is a triangle, which is
inverted depending on stem
On 2012/04/14 13:56:18, Milimetr88 wrote:
> What does solfa mean?
Solfège.  Here it was probably a typo for "The fa is a triange"

http://codereview.appspot.com/5975054/diff/1/lily/staff-symbol-referencer.cc
File lily/staff-symbol-referencer.cc (right):

http://codereview.appspot.com/5975054/diff/1/lily/staff-symbol-referencer.cc#newcode126
lily/staff-symbol-referencer.cc:126: * Returns vertical position of a
symbol relatively to the staff.
On 2012/04/14 13:56:18, Milimetr88 wrote:
> Relative to which element?
"relative to the staff"
Most English speakers think of this construction as an adjective,
describing 'position', so we use the adjective form 'relative'.
We see 'relatively' a lot from people who think in other languages, but
that makes English speakers search for the adjective being modified by
'relatively'.

http://codereview.appspot.com/5975054/diff/1/lily/staff-symbol-referencer.cc#newcode137
lily/staff-symbol-referencer.cc:137: * The unit is halves of staff
space.
On 2012/04/14 13:56:18, Milimetr88 wrote:

> Is there any official coding style for comments? I couldn't find any
in CG.

With very few exceptions, block comments in LilyPond C code use no *.
That is enough to recognize the convention.  You should take out the *s
here.

> And as it was stated here:

http://codereview.appspot.com/5651069/diff/5002/lily/note-collision.cc#newcode191
> leading *'s prevent comments misalignment.

Those are the exceptions.  Emacs' indenter will left-align block
comments. Years ago someone auto-indented the code with emacs and
destroyed all the ASCII-art comments that draw note-configurations.

If we write an explicit rule, then it is harder to adapt the next time
we meet an exceptional case.

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