On Fri, 2009-04-17 at 22:17 -0600, Carl D. Sorensen wrote: > > > On 4/17/09 1:47 PM, "joenee...@gmail.com" <joenee...@gmail.com> wrote: > > > Very pretty slurs! > > Thanks! > > > > > > > http://codereview.appspot.com/41099/diff/1021/59 > > File lily/bezier.cc (right): > > > > http://codereview.appspot.com/41099/diff/1021/59#newcode275 > > Line 275: Bezier::subdivide (Real t, Bezier &left_part, Bezier > > &right_part) > > We only use references if they are const (for clarity), so please change > > the arguments to pointers. Also, this function can be marked as const. > > So, if I understand right, you want > Bezier::subdivide (Real t, Bezier *left_part, Bezier *right_part) const
Yep > > and then in the code, instead of > > left_part.control_[i]= > > I do > > *left_part.control_[i]= > > Is this right? This should be "left_part->control_[i]=". It is also correct to write "(*left_part).control_[i]=", but the former is preferable. > > 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. Joe _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
