lilypond-devel
[Top][All Lists]
Advanced

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

Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden)


From: nine . fierce . ballads
Subject: Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden)
Date: Sat, 16 Nov 2019 10:07:37 -0800

I haven't reviewed the ly or scm.


https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh
File lily/include/measure-attached-spanner.hh (right):

https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode4
lily/include/measure-attached-spanner.hh:4: Copyright (C) 1997--2015 Jan
Nieuwenhuizen <address@hidden>
When you add a new file, I think you're supposed to use (C) <this year>
<your name>.  At least, that's what I was once told.

https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode20
lily/include/measure-attached-spanner.hh:20: #ifndef
Measure_attached_spanner_HH
all caps, please

https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode24
lily/include/measure-attached-spanner.hh:24: #include "std-vector.hh"
I don't see anything in this header that uses a vector.  Unless I'm
wrong, this should be moved to whichever cc files require it.  Same goes
for any other unnecessary headers.

https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc
File lily/measure-attached-spanner.cc (right):

https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode45
lily/measure-attached-spanner.cc:45: //Direction dir =
get_grob_direction (me);
remove

https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode53
lily/measure-attached-spanner.cc:53: Spanner *orig_spanner =
dynamic_cast<Spanner *> (me->original ());
If I understand correctly, me->original () can only ever be either null
or another instance of the same type as me; therefore, use static_cast
here.

Also, if it's logically possible for me->original () to be null in this
case, check for it before dereferencing below.

https://codereview.appspot.com/571180043/



reply via email to

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