Re: Issue 5695: reduce dynamic casting (issue 575530107 by nine.fierce.ball...@gmail.com)

[dak]

https://codereview.appspot.com/575530107/diff/581530050/lily/spaceable-grob.cc
File lily/spaceable-grob.cc (right):

https://codereview.appspot.com/575530107/diff/581530050/lily/spaceable-grob.cc#newcode92
lily/spaceable-grob.cc:92: break;
That looks still too complicated.  Just do return *next_spring; here, no
need to break out of the loop.  Then you don't need the variable spring
at all (since it will be 0 if the loop exits regularly) and can hijack
its name for the more cumbersome next_spring.  And the final return can
just be return Spring ();

Oh, and the programming_error can be called unconditionally then.

https://codereview.appspot.com/575530107/

Issue 5695: reduce dynamic casting (issue 575530107 by nine.fierce.ball...@gmail.com)

[nine . fierce . ballads]

Reviewers: ,

Description:
https://sourceforge.net/p/testlilyissues/issues/5695/

Here are a few miscellaneous things that I've been holding onto for a
while because they would have been out of context in other reviews. 
Each file is in a separate commit, though they're squashed in this
review.

1: defer dynamic_cast in Tie_column::add_tie
2: reduce unsmobbing in Spaceable_grob::get_spring
3: reduce unsmobbing in Paper_column::is_musical

Please review this at https://codereview.appspot.com/575530107/

Affected files (+14, -11 lines):
  M lily/paper-column.cc
  M lily/spaceable-grob.cc
  M lily/tie-column.cc


Index: lily/paper-column.cc
diff --git a/lily/paper-column.cc b/lily/paper-column.cc
index 
6fc65f27614e3c16cdf63766ee3642514b0afea6..7f17dbf8909dba3406ebd375dd1839c5b637e6c9
 100644
--- a/lily/paper-column.cc
+++ b/lily/paper-column.cc
@@ -108,10 +108,9 @@ bool
 Paper_column::is_musical (Grob *me)
 {
   SCM m = me->get_property ("shortest-starter-duration");
-  Moment s (0);
-  if (unsmob (m))
-s = *unsmob (m);
-  return s != Moment (0);
+  if (Moment *s = unsmob (m))
+return *s != Moment (0);
+  return false;
 }
 
 bool
Index: lily/spaceable-grob.cc
diff --git a/lily/spaceable-grob.cc b/lily/spaceable-grob.cc
index 
dae8db19c81726f9f2dd2d6039e1e230ad1cb358..95b7d10877753bd2c522c5646389b6e3eae6f2d3
 100644
--- a/lily/spaceable-grob.cc
+++ b/lily/spaceable-grob.cc
@@ -81,13 +81,17 @@ Spaceable_grob::get_spring (Paper_column *this_col, Grob 
*next_col)
   Spring *spring = 0;
 
   for (SCM s = this_col->get_object ("ideal-distances");
-   !spring && scm_is_pair (s);
-   s = scm_cdr (s))
+   scm_is_pair (s); s = scm_cdr (s))
 {
   if (scm_is_pair (scm_car (s))
-  && unsmob (scm_cdar (s)) == next_col
-  && unsmob (scm_caar (s)))
-spring = unsmob (scm_caar (s));
+  && unsmob (scm_cdar (s)) == next_col)
+{
+  if (Spring *next_spring = unsmob (scm_caar (s)))
+{
+  spring = next_spring;
+  break;
+}
+}
 }
 
   if (!spring)
Index: lily/tie-column.cc
diff --git a/lily/tie-column.cc b/lily/tie-column.cc
index 
c3e4ec6068da02d86484217820ea884a759dcaf2..4c1b2d504f0c4b35b4046634b091b1c3fa79e56b
 100644
--- a/lily/tie-column.cc
+++ b/lily/tie-column.cc
@@ -37,12 +37,12 @@ using std::vector;
 void
 Tie_column::add_tie (Grob *tc, Spanner *tie)
 {
-  Spanner *me = dynamic_cast (tc);
-
   if (tie->get_parent (Y_AXIS)
   && has_interface (tie->get_parent (Y_AXIS)))
 return;
 
+  // TODO: Change to a Spanner in the function signature.
+  Spanner *me = dynamic_cast (tc);
   if (!me->get_bound (LEFT)
   || (me->get_bound (LEFT)->get_column ()->get_rank ()
   > tie->get_bound (LEFT)->get_column ()->get_rank ()))