lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Issue 1320: Scheme bar line interface (issue 6305115)


From: mtsolo
Subject: Issue 1320: Scheme bar line interface (issue 6305115)
Date: Wed, 20 Jun 2012 19:13:01 +0000

Good work - two overall things.

1) In Scheme, try to avoid `do'.  Use map and reduce.
2) Have you tested performance hits on large scores?  It may be worth it
to leave bar-line.cc as the default and have your script as a Scheme
implementation, not unlike the way we do bezier curves.

Cheers,
MS


http://codereview.appspot.com/6305115/diff/1/lily/bar-line.cc
File lily/bar-line.cc (right):

http://codereview.appspot.com/6305115/diff/1/lily/bar-line.cc#newcode31
lily/bar-line.cc:31: #include <set>
Is it worth it to keep this file?  I'd just move what's left over to
Scheme at that point.

http://codereview.appspot.com/6305115/diff/1/lily/span-bar.cc
File lily/span-bar.cc (right):

http://codereview.appspot.com/6305115/diff/1/lily/span-bar.cc#newcode36
lily/span-bar.cc:36: Pointer_group_interface::add_grob (me,
ly_symbol2scm ("elements"), b);
Ditto - you can likely get rid of this as well.

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm#newcode80
scm/bar-line.scm:80: (cons (- height half-staff) (+ height half-staff))
(interval-widen half-staff height)

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm#newcode94
scm/bar-line.scm:94: (stencil empty-stencil))
As you use let*, you can move all the set! to redefinitions of stencil
in the let*. It seems slightly more in keeping w/ coding practice (could
be wrong, though).

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm#newcode118
scm/bar-line.scm:118:
To be more scheme-ic, try a map to get an array of stencils w/ the
correct offsets and then using reduce on the list of stencils to smoosh
them together via ly:stencil-add.

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm#newcode150
scm/bar-line.scm:150: stencil)
Ditto - as a general rule, avoid `do'. Map and reduce.

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm#newcode214
scm/bar-line.scm:214: X RIGHT colon-stil kern))
Can you turn the above into the result of a call to a recursive function
that uses ly:stencil-combine-at-edge?

http://codereview.appspot.com/6305115/diff/1/scm/bar-line.scm#newcode495
scm/bar-line.scm:495: ;; bulky side. Rewritten by Han-Wen. Ported from
c++ to Scheme by Marc Hohl.
Can you get this down to the 80 character line width?

http://codereview.appspot.com/6305115/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]