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/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
