Re: Issue 5362: Generalize context searches (issue 344050043 by address@hidden)

[nine . fierce . ballads]

(initial feedback—more later)


https://codereview.appspot.com/344050043/diff/1/lily/global-context.cc
File lily/global-context.cc (right):

https://codereview.appspot.com/344050043/diff/1/lily/global-context.cc#newcode143
lily/global-context.cc:143: return score;
On 2018/07/01 13:21:42, dak wrote:
> So if there already is a Score context, it will be returned in lieu of
_any_
> request for creating any kind of context?  That sounds weird.

The missing information is that this method is called to create an
immediate child after acceptability has been verified, so it is only a
Score.

The assumption that Global has a single child which is a Score (well,
that if Global has a Score, no other children are of interest, which is
slightly different), was present in the old code (see the first few
lines of of create_unique_context and find_create context).  I wouldn't
call this any worse, but a comment would still be beneficial.

https://codereview.appspot.com/344050043/diff/1/lily/include/context.hh
File lily/include/context.hh (right):

https://codereview.appspot.com/344050043/diff/1/lily/include/context.hh#newcode61
lily/include/context.hh:61: template <FindMode mode, Direction dir>
On 2018/07/01 13:21:42, dak wrote:
> Since the dir argument is actually being read from the command in
> question rather than being fixed at compile time, you then need to
hardcode a
> farmout to the various variants since the actually called variant
cannot, after
> all, be determined at compile time in general.

You're focusing on the context-specced music iterator which has the
runtime direction.  There are other callers using a hard-coded constant
direction.

This is something I vacillated about.  At first, I had 7 public methods
(2 modes do something consistent but not really useful) with direction
in the method name.  That has long been my preference over providing
constant arguments.  Then I got to the context-specced music iterator
and thought, maybe I should go the other way in this case, inline the
switch(dir) and trust the compiler to optimize it in the cases that
direction is known at compile time.

> That's a lot of complication for no discernible gain.  Why not pass
the
> respective parameters into the function?  Would that cause some kind
of problem?

I don't know.  In terms of meaning, I have no problem with it.  I'll do
it and post an update.

https://codereview.appspot.com/344050043/