PartCombine: Keep track of the state in the Part_combine_engraver (issue3334043)

[v . villenave]

Hi Reinhold,
I really like where you're going with this! As always, I'm not a coder
(yet) so this is just my naive point of view.




http://codereview.appspot.com/3334043/diff/1/lily/part-combine-engraver.cc
File lily/part-combine-engraver.cc (right):

http://codereview.appspot.com/3334043/diff/1/lily/part-combine-engraver.cc#newcode52
lily/part-combine-engraver.cc:52: bool item_to_create_;
if it's really a grob, then why are you calling it "item"?

http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc
File lily/part-combine-iterator.cc (right):

http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode69
lily/part-combine-iterator.cc:69: Stream_event *unisono_event_;
How about declaring these variables on the same line? (I'm not sure this
style is used in Lily's source code, though.)

http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode110
lily/part-combine-iterator.cc:110: void broadcast_new_state
(Music_iterator *new_active, Stream_event *new_state_event);
I'm not familiar with C++ coding style, but that looks like an awfully
long line.

http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode133
lily/part-combine-iterator.cc:133: mmrest_event_ = 0;
Since all these variables are used together a lot, another possibility
would be to use an array? (At least that's what I've been reading, but
I'm not sure it's worth considering :)

http://codereview.appspot.com/3334043/diff/1/scm/define-music-types.scm
File scm/define-music-types.scm (right):

http://codereview.appspot.com/3334043/diff/1/scm/define-music-types.scm#newcode37
scm/define-music-types.scm:37: (ApartEvent
I find this name a bit hard to understand. Is ApartEvent some kind of a
voice-agnostic solo-event, that can either be a solo-one- or
solo-two-event? (in which case "SoloEvent" could be a better name?)

(Now that I'm thinking about it, in the future it could be interesting
to have a single solo-event kind, that could take a "thread-number"
event-property such as 'one or 'two, which would then be passed to the
iterator so that any event could be forced to be regarded as a solo-one-
or solo-two-event...
Or maybe even make the iterator able to take any arbitrary number of
music "threads", not just two... Ok, I'm dreaming here.)

http://codereview.appspot.com/3334043/