Re: Checks to see if tuplet brackets have bounds (issue 13582046)

[dak]

On 2013/09/13 08:41:42, Trevor Daniels wrote:
> On 2013/09/13 07:09:44, mike7 wrote:

> > With respect to your point about null pointers and the nature of the
patch, I
> > agree that there needs to be a better way to handle this.  To me,
the general
> > problem seems to be "what do we do when we assume a grob will have
something
> > (bound, object, etc.) and it doesn't?"  Is the solution to suicide
the spanner
> > if it doesn't have bounds?  Is the solution to raise a programming
error like
> we
> > do now (I prefer that solution)?

> LilyPond should always try to continue when an error is detected.
> If the error is thought to be a user error a warning should be
> issued and an assumption made about what was intended.  If the
> error is thought to be due to faulty or missing code (as here) the
> erroneous operation should be abandoned and a programming error
> issued.  Reported programming errors should always be recorded in
> an issue tracker unless one already exists.  At least that, I
> believe, was the general approach adopted in the past and it seems
> sound to me.

Well, being defensive is always a good idea.  It's just that when we get
to _know_ the defenses to be triggered without us expecting it, we
should make use of the opportunity to analyze what is going wrong here.

Mike says that the change is due to more crossstaff checks happening
now.  I have no better guess here, but the issue description of issue
3199 does not at all mention crossstaff, and the error does not occur in
a context or in code mentioning crossstaff.  That presumably means that
the code and/or the issue is factored lousily, resulting in problems
triggered in/by mostly unrelated code and circumstances.  And while we
are adding additional safety checks, we better think about what we can
do to improve this situation.  Because afterwards, the incentive for
thinking about it will have disappeared.

The main question we need to ask ourselves is: does the condition for
which we do an early return here occur in circumstances where an early
return is a _correct_ answer?  If not, we should be raising a
programming error here unless the situation can _only_ occur in
circumstances for which we already get a programming error (but then we
should not be calling the function in the first place, should we?).

https://codereview.appspot.com/13582046/