Re: Improve implementation of dashed slurs

[Carl D. Sorensen]

On 4/18/09 7:56 PM, "Joe Neeman" <address@hidden> wrote:

> On Fri, 2009-04-17 at 22:17 -0600, Carl D. Sorensen wrote:
> 
>>> http://codereview.appspot.com/41099/diff/1021/62#newcode347
>>> Line 347: SCM dash_details)
>>> Since dash_details seems to just be a list of Reals, perhaps its better
>>> to have a vector<Real> const& (with an empty vector to signify a solid
>>> slur).
>> 
>> No, dash_details is a list of lists of floating point numbers.  There is one
>> list for each segment of the slur; there can be as many segments as the user
>> desires.
> 
> Ah, OK. I won't insist, but I think it might be nicer to make this more
> statically typed. For example:
> typedef struct {
>   Real t_min;
>   Real t_max;
>   Real dash_fraction;
>   Real dash_period;
> } Dash_definition;
> and then make the argument a vector<Dash_definition>. Since C++ is a
> statically typed language, I think it's nicer to make use of that static
> typing and do the type checking/conversion for SCM variables as early as
> possible instead of propagating them through the C++ code.

I actually like the idea of checking it early, because then I could throw a
meaningful error if there's a problem.  However, if I do that, I'll have to
parse the SCM list in three different places:  slur::print, tie::print, and
arpeggio::print.  If I let it be a SCM, then I only parse it one place:
Lookup::slur.  I thought that the lack of duplicated code might balance out
with the lack of static typing.

If I do change to static typing, then I should have one routine that is
called from all three places, something like scm2dash_definition.  If I were
to write such a function, where would I put it?  It seems like
lily/lily-guile.cc is supposed to contain more general things than this
would be.

I could make it part of Lookup, and call it there, but it's not really
consistent with a Lookup:: function, since they're supposed to be stencils.
I could put it in lily/misc.cc, but it doesn't seem much like those
functions.

How about if I added it to Slur::, so we had a Slur::parse_dash_definition,
and then I could call it from Slur::print, Tie::print, and Arpeggio::print?
That seems to make sense to me.

Thanks,

Carl