On Sat, 2009-04-18 at 21:08 -0600, Carl D. Sorensen wrote: > > > On 4/18/09 7:56 PM, "Joe Neeman" <joenee...@gmail.com> 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.
Of those options, I prefer lily-guile.cc because Dash_definition is probably useful more generally than just for slurs (maybe it will be used for line spanners in the future?). Another option would be to make Dash_definition a smob (like Pitch, for example) and provide a ly:make-dash-definition function. This has the disadvantage, though, that it makes the ly code more verbose: \override Slur #'dash-definition = `(,(ly:make-dash-definition 0 0.25 1 1) ,(ly:make-dash-definition 0.3 0.7 0.4 0.75) etc..) Joe _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
