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

[dak]

Without wanting to rain on your parade: what is all this about?  I
actually had been planning to fix this collision stuff: there are
several fundamental shortcomings.  The code can't deal sensibly with
different noteheads (harmonics, mixed duration chords which are required
for some string music) as it picks some information just off the first
notehead.  It does not cleanly distinguish stem duration and notehead
duration.  It uses rounded notehead positions for its decisions instead
of the actual notehead positions (possibly shifted, partly as a
consequence of optical corrections).  It does not compare all the
relevant data of noteheads to be merged, nor does it take into account
different merging possibilities.

All of this has been implemented with ad-hoc variables and ad-hoc code.

It does appear that you now "C++ize" both variables and code, creating
artifical APIs for something that is, at its heart, just ad-hoc.

This makes it harder to actually fix the deficiencies of the code.  I
don't see that you address the deficiencies: instead you change the
style to a style that is harder to maintain and understand because now,
to change its works, you need to look at the newly introduced APIs and
change those as well as its uses, for code and data that is actually
quite localized.

What is the gain from this additional layering?  The way it looks to me,
this will make the code harder to maintain (and fix) than it was
previously.  Can you describe the motivation behind your work?

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