Why RTTI is considered as a bad design?

[davidkevin]

Hi,
what is a reason for which RTTI is considered as a bad design?
Stroustrup has written in his book TC++PL that the most common case of usage RTTI technique is with switch instruction, when one want to decide what code should be executed basing on what is a "real" type of a passed object. There is given an example in which an object of shape class was passed to the function and different actions were executed depending on that whether a shape is circle, square, triangle, etc. He has written that such construction is a sign that one should replace the switch-case sequence by virtual functions.

In my opinion such dealing is reasonable if what should be done is really amatter of given type (for example counting the area of circle).

But is RTTI design undesirable in the situation in which a choice of actiondepends upon how an external module want to treat objects basing on types of them?

For example if one is making a space-war game in which there are many kindsof ships usage of RTTI seems quite logical to me.

 

[Marcel Müller]

what is a reason for which RTTI is considered as a bad design?
Stroustrup has written in his book TC++PL that the most common case of usage RTTI technique is with switch instruction, when one want to decide what code should be executed basing on what is a "real" type of a passed object. There is given an example in which an object of shape class was passed to the function and different actions were executed depending on that whether a shape is circle, square, triangle, etc. He has written that such construction is a sign that one should replace the switch-case sequence by virtual functions.

Well, this is not always possible. Think of multiple dispatch for instance.

But is RTTI design undesirable in the situation in which a choice of action depends upon how an external module want to treat objects basing on types of them?

For example if one is making a space-war game in which there are many kinds of ships usage of RTTI seems quite logical to me.

I don't see directly why RTTI should help here. But if you refer to
interaction between different types of ships, the yes, this is again a
multiple dispatch problem.


Marcel

 

[Alain Ketterlin]

what is a reason for which RTTI is considered as a bad design?
Stroustrup has written in his book TC++PL that the most common case of
usage RTTI technique is with switch instruction, when one want to
decide what code should be executed basing on what is a "real" type of
a passed object. There is given an example in which an object of shape
class was passed to the function and different actions were executed
depending on that whether a shape is circle, square, triangle, etc. He
has written that such construction is a sign that one should replace
the switch-case sequence by virtual functions.

In my opinion such dealing is reasonable if what should be done is
really a matter of given type (for example counting the area of
circle).

Using class/inheritance/virtual functions provides static information
(used by the compiler). RTTI is, well, run-time.

Suppose you add a new shape sub-class in your example. Using RTTI, the
compiler will be unable to tell you that you forgot to add the
corresponding case in your switch. If you use virtual functions, the
compiler will tell you that your new class fails to implement the
function.

But is RTTI design undesirable in the situation in which a choice of
action depends upon how an external module want to treat objects
basing on types of them?

I can't parse your remark here. If you mean that an external module may
need dynamic type information on your objects to be able to handle them
properly, then why not provide the correct functions in your own
classes?

For example if one is making a space-war game in which there are many
kinds of ships usage of RTTI seems quite logical to me.

How can you be sure all kinds of ships are correctly handled by all of
your code? You can't. You have to manually inspect all switches.

-- Alain.

 

[davidkevin]

W dniu środa, 1 sierpnia 2012 10:57:06 UTC+2 użytkownik Alain Ketterlin napisał:

(e-mail address removed) writes:
I can't parse your remark here. If you mean that an external module may
need dynamic type information on your objects to be able to handle them
properly, then why not provide the correct functions in your own
classes?

Because such representation could be completely inconsistent with a problemwe
are trying to solve. If we assume that every ship choices a strategy of fight respecting if an enemy object is heavy-attack, mobile, supervisor or other ship then including data how object X should fight with object Y in object Y make the design simply unusually strange. And probably a situation in which managing such code will be impossible or very hard will appear soon.

How can you be sure all kinds of ships are correctly handled by all of
your code? You can't. You have to manually inspect all switches.

Yes. But note that in the fact it is a case in which you have to check it manually anyway. If data about objects of given classes are scattered in other classes then things are undoubtedly worse.

A solution here could be that each definition of class representing the ship is hold in one file (or one directory) which preferably has no other dataand information about what are a types of ships in a game is extracted by a script.

 

[Alain Ketterlin]

W dniu środa, 1 sierpnia 2012 10:57:06 UTC+2 użytkownik Alain Ketterlin napisał:

Because such representation could be completely inconsistent with a
problem we are trying to solve. If we assume that every ship choices a
strategy of fight respecting if an enemy object is heavy-attack,
mobile, supervisor or other ship then including data how object X
should fight with object Y in object Y make the design simply
unusually strange. And probably a situation in which managing such
code will be impossible or very hard will appear soon.

OK, that's what Marcel mentionned in another message: you need multiple
dispatch (http://en.wikipedia.org/wiki/Multiple_dispatch).

Yes. But note that in the fact it is a case in which you have to check
it manually anyway. If data about objects of given classes are
scattered in other classes then things are undoubtedly worse.

A solution here could be that each definition of class representing
the ship is hold in one file (or one directory) which preferably has
no other data and information about what are a types of ships in a
game is extracted by a script.

If I were you, I would use an enum enumerating the classes, and abstract
away the strategy in a distinct class. At least your nested switches
would be checked for completeness.

-- Alain.

 

[Noah Roberts]

Hi,

what is a reason for which RTTI is considered as a bad design?

Stroustrup has written in his book TC++PL that the most common case of usage RTTI technique is with switch instruction, when one want to decide whatcode should be executed basing on what is a "real" type of a passed object.. There is given an example in which an object of shape class was passed to the function and different actions were executed depending on that whether a shape is circle, square, triangle, etc. He has written that such construction is a sign that one should replace the switch-case sequence by virtual functions.

See Liskov Substitution Principle: http://en.wikipedia.org/wiki/Liskov_substitution_principle

If your code looks like so:

if (type_1 * ptr = dynamic_cast<type_1*>(somePtr))
....
else if (type_2 * ptr = dynamic_cast<type_2*>(somePtr))
....

You're violating the principle. What this means is that all places that look like this will have to be searched for, found, and modified if you add anew type behind the abstraction. This can quite rapidly explode into a maintenance nightmare and cause bugs that are difficult and/or impossible to find.

This doesn't mean that RTTI in and of itself is bad, only that this kind ofuse of it is bad. For a good use of RTTI review Alexandrescu's visitor ormultiple-dispatch system. These systems do not fall prey to the same kinds of problems because they either do not violate LSP at all (because they're a chain of responsibility that forwards the type on if the check fails) or do so in a localized abstraction that can be reused and hide the facts from all the client code that would otherwise be fragile.

 

[Nick Keighley]

See Liskov Substitution Principle:http://en.wikipedia.org/wiki/Liskov_substitution_principle

If your code looks like so:

if (type_1 * ptr = dynamic_cast<type_1*>(somePtr))
...
else if (type_2 * ptr = dynamic_cast<type_2*>(somePtr))
...

You're violating the principle.

in what way are you violating it? And yes I did read the wikipedia
article

 What this means is that all places that look like this will have to besearched for, found, and modified if you add a new type behind the abstraction.  This can quite rapidly explode into a maintenance nightmare and cause bugs that are difficult and/or impossible to find.

yes you've kind of thrown away the whole point of polymorphism. As a
rule of thumb look with suspicion at any 'switch' (which may be
implemented as a bunch of 'if's) that appears in a C++ program.
'Factory' patterns are one honourable exception.

 

[Noah Roberts]

in what way are you violating it? And yes I did read the wikipedia

article

The LSP says that if your function F takes type A and you pass in a type B then B should behave exactly as if it where an A. If F has to perform downcasting then A is not providing the correct abstraction and B, in some way,behaves in ways incompatible with A as it relates to F.

There are some exceptions, such as when a chain of responsibility is set up, but generally speaking if your code is downcasting to various raw types and then doing stuff you're violating LSP. If you're not violating LSP you may still be violating the Open-Closed principle.

A better illustration than I provided is the Rectangle/Square problem.

yes you've kind of thrown away the whole point of polymorphism.

Exactly.

 

[Öö Tiib]

See Liskov Substitution Principle: http://en.wikipedia.org/wiki/Liskov_substitution_principle


If your code looks like so:


if (type_1 * ptr = dynamic_cast<type_1*>(somePtr))
...
else if (type_2 * ptr = dynamic_cast<type_2*>(somePtr))
...

You're violating the principle. What this means is that all places that look like this will have to be searched for, found, and modified if you adda new type behind the abstraction. This can quite rapidly explode into a maintenance nightmare and cause bugs that are difficult and/or impossible to find.

Usually i see dynamic_cast<>() more used for cross-casting rather than down-casting. I bring an example. A specific Door passed to SomeDoer:pen(Door*) may have Lockable interface or not. To have all Doors Lockable by default would deviate from sane design since most doors can not be locked. Success of unlocking may depend of availability of Keys or LockPicks or whatever for this SomeDoer so open() forwards these problems to SomeDoer::unlock(Lockable*):

bool SomeDoer:pen( Door* door ) const
{
// we might need to unlock first on some cases
Lockable* lock = dynamic_cast<Lockable*>(door);
if ( lock != NULL && lock->isLocked() && !unlock( lock ) )
{
return false;
}

return door->open();
}

Can you tell how such a situation can be resolved as elegantly without RTTI(or some self-made substitution of RTTI)?

 

[Noah Roberts]

The ->unlock() method becomes a no-op on a non-lockable door. In other words,

one may always call unlock, and if the door is lockable, it will be unlocked;

if not, no-op.

I'm not a big fan of adding functions to interfaces for subclasses or for possibly implementable interfaces. I like the RTTI method better.

LSP is just a principle and there are degrees of breaking it. This seems like a fairly reasonable one, though I might consider having a "maybe_unlock" function that does the casting (it's more reusable).

There's a balance here though. Just because there's a violation of LSP doesn't mean the code is bad, it just means that it could be and in a specificway. If you suddenly needed to make some new door type that implemented adifferent interface such that this open door function no longer worked right then you're starting to run afoul of the LSP in bad ways.

An alternative to this kind of thing is a visitor. There could be a door_opener visitor that takes a lockable and a non-lockable door as parameters. In many cases this is a much better alternative to either using RTTI or noop functions in base classes that don't belong. For this simple of an issue though it could be very tempting just to violate LSP and call it good. The minus here is that IFF something changes you didn't think would (and this happens a LOT) then you're kind of screwing yourself by being lazy. But it's a balancing act.

 

[Öö Tiib]

The ->unlock() method becomes a no-op on a non-lockable door. In other words,
one may always call unlock, and if the door is lockable, it will be unlocked;
if not, no-op.

Unlocking itself (with no parameters whatsoever)? It can no way made a responsibility of a door. If there is lock then it is real and unlocking it may fail without proper keys or pass-codes. A generic door knows (and should know) nothing of it.

Only the cross-casting into "Lockable" may be (and often is) made responsibility of a "Door" but bloating its interface with fake complexities of possibly "Lockable" is bad design.

 

[goran.pusic]

Anything in software can be "improved" (for some measure of the word), withanother level of abstraction. In this case, you're handling a case of a door being lockable. But if you introduce another level of abstraction, you might say "well, I don't care if this door is lockable, or there is a security check that must be done before opening, or you have to un-jam the door before opening, or..., I only care about performing an action before openingthe door". IF you look at things this way, you might end up with e.g.

void open_door_ex(door& d, door_pre_opener& preopen)
{
preopen(d);
door.open();
}

.... where preopen is created "with" the door (and a "no-op", or null, preopener is used with a "normal" door).

And you don't need RTTI anymore (albeit, here, at the expense of carrying more context around).

Is the above a better solution than the "lockable" interface? I say, only time will tell. If there is another "preopen" action at some point, perhaps an abstraction is in order. If not, no, not really.

( NB: I don't like seeing a pointer parameter that's not checked for null, as well as success/fail returns from "action" methods like door:pen )

Goran.

 

[Öö Tiib]

void open_door_ex(door& d, door_pre_opener& preopen)
{
preopen(d);

door.open();
}

... where preopen is created "with" the door (and a "no-op", or null, preopener is used with a "normal" door).

And you don't need RTTI anymore (albeit, here, at the expense of carryingmore context around).

The preopener class is additional complexity. There are several ways how topay by being less efficient and more complex in such situations and get rid of usage of RTTI. I see no point really.

Is the above a better solution than the "lockable" interface? I say, onlytime will tell. If there is another "preopen" action at some point, perhaps an abstraction is in order. If not, no, not really.

I usually tend to keep simple things simple and add complex solution only when that "some point in time" arrives and problem gets more complex. Premature complex solution for a simple problem is called over-engineering. It isusually making things worse since it usually does predict the future wrongly.

For example there may be more possible obstacles to take care in process ofopening future "door" (like whole pile of "barricades" to remove, "nails" to pull out and "locks" to unlock). But then the whole design might change so the "lockability" is not a property of whole "door" itself anymore but it's (possibly several) components and the preopener targets a wrong thing then.

BTW separate "lockable" interface makes sense immediately if same interfaceis used for some "non-doors" as well. If only "doors" may be "lockable" then it might be better to have some "LockableDoor" subclass for simplicity and split it in future when the need arrives.

( NB: I don't like seeing a pointer parameter that's not checked for null, as well as success/fail returns from "action" methods like door:pen )

Verification of possible interface contracts/preconditions was removed fromoriginal example since it was a short illustrative example, not real production code.

Indicating outcome with return value for an action that may have several outcomes is quite common idiom. What is wrong with it? Like for example std::set erase: size_type erase( const key_type& x ); It returns count of erasedelements, since there may be that such thing was not in set.

 

[Noah Roberts]

Unlocking itself (with no parameters whatsoever)? It can no way made a responsibility of a door. If there is lock then it is real and unlocking it may fail without proper keys or pass-codes. A generic door knows (and shouldknow) nothing of it.



Only the cross-casting into "Lockable" may be (and often is) made responsibility of a "Door" but bloating its interface with fake complexities of possibly "Lockable" is bad design.

Actually, it seems to me that composition is better here. A door either has a lock or doesn't. That lock is either engaged or it isn't. Opening a door is either successful or it isn't (and being locked isn't the only reason it might fail--could be a busted knob or something in the way). A door that does not have a lock could have one added to it later in many differentways.

Thus cross casting seems like a bad solution and instead there should be a failable function to attempt an open, a function that gets the lock or listof locks (could be many), and then the client can look for locks, check their state, and open the door. No RTTI required. This seems to me to be the most logical solution given the problem domain.

 

[Öö Tiib]

Actually, it seems to me that composition is better here. A door either has a lock or doesn't. That lock is either engaged or it isn't. Opening adoor is either successful or it isn't (and being locked isn't the only reason it might fail--could be a busted knob or something in the way). A doorthat does not have a lock could have one added to it later in many different ways.

Yes. Refactoring inheritance into composition is the next step if needed. That happens when there is a need to change lockability dynamically during doors lifetime. Not reasonable before, because it adds some data to all Doors (a pointer to Lock). It is easy to refactor if the cross-casting into "Lockable" is done by some "Door::getLock()" because user of the interface does not care if it got interface to the same object or its component.

Thus cross casting seems like a bad solution and instead there should be a failable function to attempt an open, a function that gets the lock or list of locks (could be many), and then the client can look for locks, check their state, and open the door. No RTTI required. This seems to me to be the most logical solution given the problem domain.

List of locks is again the next step of refactoring if it is needed. What iam saying is that it is clearly over-engineering to make all doors with list of locks when it is required that only few of them are non-dynamically lockable with a single key. The need for additional flexibility may never happen but the additional complexity will be there forever.

 

[Noah Roberts]

Yes. Refactoring inheritance into composition is the next step if needed.

When starting from scratch, refactoring inheritance into composition is nota next step. Always prefer composition to inheritance where it makes sense. This isn't over-engineering, it's engineering to just the right level. As soon as you need some doors to have locks and others to not you should design a solution appropriate to that force of change. There's no good reason to inherit a "lockable" interface when composition nicely does the job quite a bit better.

A null pointer is not a large price to pay for this level of extensibility offered by the adherence to a basic design principle. If it is then it is quite simple to have a set of subclasses behind the door interface where one always returns an empty lock set without having a variable for it. Unlike the solution of having an "unlock()" method in a door that has no locks, having a "get_locks()" function instead makes perfect sense and can return empty.

When design principles all fall neatly into place like this while still reflecting the problem domain better it is not "over-engineering" to prefer them to a less extensible construction. I can't see a good reason to prefer a less fluid design that offers no concrete benefits whatsoever. It's evenlikely to be a faster setup.

Now, if the inheritance relationship already exists and there's nothing forcing a change to composition it's not a prudent choice to refactor it untilthere is. That's not really what we're talking about here though.

 

[Andrew Cooper]

<snip>
Anything in software can be "improved" (for some measure of the word), with another level of abstraction.
<snip>

With the exception of the problem of too many layers of indirection
(which invariably happens as soon as the word 'enterprise' gets added to
a project)

~Andrew

 

[Öö Tiib]

When starting from scratch, refactoring inheritance into composition is not a next step. Always prefer composition to inheritance where it makes sense. This isn't over-engineering, it's engineering to just the right level.. As soon as you need some doors to have locks and others to not you should design a solution appropriate to that force of change. There's no good reason to inherit a "lockable" interface when composition nicely does the job quite a bit better.

Within given example and when designing from scratch you are right. Lock iseasy to see as optional component of most doors.

A null pointer is not a large price to pay for this level of extensibility offered by the adherence to a basic design principle. If it is then it is quite simple to have a set of subclasses behind the door interface where one always returns an empty lock set without having a variable for it. Unlike the solution of having an "unlock()" method in a door that has no locks, having a "get_locks()" function instead makes perfect sense and can return empty.

Rest of it makes sense but get_locks() makes interface that forces users ofit to assume that there may be lot of them and iterate there. When actually either one or none are required then it adds unneeded complexity?

When design principles all fall neatly into place like this while still reflecting the problem domain better it is not "over-engineering" to prefer them to a less extensible construction. I can't see a good reason to prefer a less fluid design that offers no concrete benefits whatsoever. It's even likely to be a faster setup.

No present flexibility comes free of charge. Technically one can get rid ofall inheritance just by using composition always but in practice it is terribly expensive. One can have collection of member instead of one always ifthere is slightest chance that one day there may be several needed but that is expensive too. So I still design as rigid, simple and robust as possible. Simplicity adds to flexibility to modify it in the future when needed.

 

[goran.pusic]

The preopener class is additional complexity. There are several ways how to pay by being less efficient and more complex in such situations and get rid of usage of RTTI. I see no point really.






I usually tend to keep simple things simple and add complex solution onlywhen that "some point in time" arrives and problem gets more complex. Premature complex solution for a simple problem is called over-engineering. It is usually making things worse since it usually does predict the future wrongly.

My comment is much more subtle from the get-go than what you make it out tobe. Notice how I completely avoided to say "what I suggest is better". In fact, notice that I explicitly said "no, not really". I wanted to offer an example of why/when another approach is better. The thing you seem to see is "he took a stab on my RTTI". Well, now...

Verification of possible interface contracts/preconditions was removed from original example since it was a short illustrative example, not real production code.

Yeah, precondition checking is my beef. More often than not, people pass pointers in, and then make their own life complicated by checking for NULL, signalling errors and whatnot, whereas the reality is that the callee doesn't want NULL, ever. Your example was a poster child of that. There was no need for a pointer in code, why did you even put it? To type more?

Indicating outcome with return value for an action that may have several outcomes is quite common idiom. What is wrong with it? Like for example std::set erase: size_type erase( const key_type& x ); It returns count of erased elements, since there may be that such thing was not in set.

What's wrong is that with door opening, you know that it didn't work, but you don't know why. That's often bad. Further, it's possible that any of these operations can fail for other reasons, and throw. If so, the calee has to handle failures in two different ways, which he might not be interested in doing. Comparison with set::erase unfair, because (guessing follows) is part of a very basic library that tries to be very complete. On one hand, itmight be that someone does want to know that erase "failed" because there was no x in set (I don't remember doing it; do you?), but given that erase is typically a cleanup operation, an exception is inappropriate. We know nothing of that in the door example, so it's just code that serves no visiblepurpose and gets in the way of the example.

Goran.

 

[Öö Tiib]

My comment is much more subtle from the get-go than what you make it out to be. Notice how I completely avoided to say "what I suggest is better". In fact, notice that I explicitly said "no, not really". I wanted to offer an example of why/when another approach is better. The thing you seem to seeis "he took a stab on my RTTI". Well, now...

You misunderstood me. I was initially actually asking how to get rid of common usage of RTTI i see in code. It is not "my RTTI". Thanks for suggestinga way. What i criticize here is that i feel that it is making things more complex and so i can not use it as generic solution against RTTI, thanks.

Yeah, precondition checking is my beef. More often than not, people pass pointers in, and then make their own life complicated by checking for NULL,signalling errors and whatnot, whereas the reality is that the callee doesn't want NULL, ever. Your example was a poster child of that. There was no need for a pointer in code, why did you even put it? To type more?

There are couple of static code analysis tools that warn on exactly that case. Microsoft's prefast for example. Just run it once in a while (or if youuse continuous integration then on each commit) and people stop dereferencing NULL pointers in your code base.

I used a pointer because failed dynamic_cast of references throws and so i had to type less (no try-catch needed).

What's wrong is that with door opening, you know that it didn't work, butyou don't know why. That's often bad. Further, it's possible that any of these operations can fail for other reasons, and throw. If so, the calee hasto handle failures in two different ways, which he might not be interestedin doing. Comparison with set::erase unfair, because (guessing follows) ispart of a very basic library that tries to be very complete. On one hand, it might be that someone does want to know that erase "failed" because there was no x in set (I don't remember doing it; do you?), but given that erase is typically a cleanup operation, an exception is inappropriate. We know nothing of that in the door example, so it's just code that serves no visible purpose and gets in the way of the example.

There may be is really deep logic with a door in some problem domain like that it may become offended and agressive because of attempt to open it. As with example given i assumed that the problem domain is more natural, wheresomeone attempting to open a door wants only to know if it is now opened or if it is not (no other states in that domain). Exceptions were not used because failed attempt to open a door was not exceptional in that domain. That was again just an irrelevant part of example.