On 2019/11/26 08:25:04, lemzwerg wrote:
LGTM. The goal is to have less dynamic casts, right?
Yes, and most of all, fewer cases like this:
dynamic_cast<Paper_column *> (c[j])->set_system (system);
Dynamic casting is for casting to a derived type when the actual type of
the object is not known at compile time. It returns a null pointer in
the case that casting is inappropriate. If it returns a null pointer
and you dereference it, you have made a fatal error; so, one can say two
things about this example. If the programmer knew that it is impossible
for c[j] to refer to anything other than a (or a kind of) Paper_column,
he should have used static_cast[1], which has a lower run-time cost. If
he knew that it is possible for c[j] to refer to something else, then he
should have checked the pointer.
A maintainer coming across this has a choice. A conservative maintainer
with limited time to investigate would probably choose to check the
pointer and slow down LilyPond.
My goal is to preserve more type information in the C++ domain so that
such questionable code is much less likely to exist.
[1] Even static_cast is not ideal, because the programmer's "knowledge"
might be an improperly justified, false belief; or it might have been
true at one time, in a context that has since changed. It is better to
use the compiler's type checking as much as is practical.
P.S. This might have come across a bit ranty--it is--but I don't intend
to demean any LilyPond contributors, just to improve things.
https://codereview.appspot.com/545280043/
