Why RTTI is considered as a bad design?

[david...@o2.pl]

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 a matter of given type (for example counting the area of circle).

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.

[Marcel Müller]

On 01.08.2012 10:23, david...@o2.pl wrote:
> 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]

david...@o2.pl writes:

> 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.

[david...@o2.pl]

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

> david...@o2.pl 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 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.

>> 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.

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.

[Alain Ketterlin]

david...@o2.pl writes:

> W dniu środa, 1 sierpnia 2012 10:57:06 UTC+2 użytkownik Alain Ketterlin napisał:
>> david...@o2.pl 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
> 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).

>>> 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.
>
> 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]

On Wednesday, August 1, 2012 1:23:58 AM UTC-7, (unknown) wrote:
> 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.

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 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.

This doesn't mean that RTTI in and of itself is bad, only that this kind of use of it is bad. For a good use of RTTI review Alexandrescu's visitor or multiple-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]

On Aug 6, 7:43 pm, Noah Roberts <roberts.n...@gmail.com> wrote:
> On Wednesday, August 1, 2012 1:23:58 AM UTC-7, (unknown) wrote:
> > 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.
>
> 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 be searched 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]

On Tuesday, August 7, 2012 12:24:09 AM UTC-7, Nick Keighley wrote:
> On Aug 6, 7:43 pm, Noah Roberts <roberts.n...@gmail.com> wrote:
>
> > On Wednesday, August 1, 2012 1:23:58 AM UTC-7, (unknown) wrote:
>
> > > 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.
>
> >
>
> > 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

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.

>
>
>
> >  What this means is that all places that look like this will have to be searched 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.

Exactly.

[Öö Tiib]

On Monday, August 6, 2012 9:43:24 PM UTC+3, Noah Roberts wrote:
> On Wednesday, August 1, 2012 1:23:58 AM UTC-7, (unknown) wrote:
>
> > 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.
>
> 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 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.
>

Usually i see dynamic_cast<>() more used for cross-casting rather than down-casting. I bring an example. A specific Door passed to SomeDoer::open(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::open( 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)?

[Scott Lurndal]

=?ISO-8859-1?Q?=D6=F6_Tiib?= <oot...@hot.ee> writes:

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

>ckable*):
>
> bool SomeDoer::open( Door* door ) const
> {
> // we might need to unlock first on some cases

> Lockable* lock =3D dynamic_cast<Lockable*>(door);
> if ( lock !=3D 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)?=20

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.

[Noah Roberts]

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 specific way. If you suddenly needed to make some new door type that implemented a different 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]

On Tuesday, August 21, 2012 4:57:06 PM UTC+3, Scott Lurndal wrote:
> =?ISO-8859-1?Q?=D6=F6_Tiib?= <oot...@hot.ee> writes:
>
> >Usually i see dynamic_cast<>() more used for cross-casting rather than down=
> >-casting. I bring an example. A specific Door passed to SomeDoer::open(Door=
> >*) may have Lockable interface or not. To have all Doors Lockable by defaul=
> >t would deviate from sane design since most doors can not be locked. Succes=
> >s of unlocking may depend of availability of Keys or LockPicks or whatever =
> >for this SomeDoer so open() forwards these problems to SomeDoer::unlock(Lo=
> >ckable*):
> >
> > bool SomeDoer::open( 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)?
>
>

> 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...@gmail.com]

On Tuesday, August 21, 2012 1:11:06 AM UTC+2, Öö Tiib wrote:
> On Monday, August 6, 2012 9:43:24 PM UTC+3, Noah Roberts wrote: > On Wednesday, August 1, 2012 1:23:58 AM UTC-7, (unknown) wrote: > > > 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. > > 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 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. > Usually i see dynamic_cast<>() more used for cross-casting rather than down-casting. I bring an example. A specific Door passed to SomeDoer::open(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::open( 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)?

Anything in software can be "improved" (for some measure of the word), with another 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 opening the 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::open :-) )

Goran.

[Öö Tiib]

On Wednesday, August 22, 2012 3:13:33 PM UTC+3, (unknown) wrote:
>
>
>
> 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).

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.

> 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.

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 is usually making things worse since it usually does predict the future wrongly.

For example there may be more possible obstacles to take care in process of opening 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 interface is 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::open :-) )

Verification of possible interface contracts/preconditions was removed from original 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 erased elements, since there may be that such thing was not in set.

[Noah Roberts]

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 different ways.

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.

[Öö Tiib]

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 i am 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]

On Wednesday, August 22, 2012 11:32:26 AM UTC-7, Öö Tiib wrote:
> On Wednesday, August 22, 2012 8:28:55 PM UTC+3, Noah Roberts wrote:

> > 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 different ways.
>
>
>
> Yes. Refactoring inheritance into composition is the next step if needed.

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.

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 even likely 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 until there is. That's not really what we're talking about here though.

[Andrew Cooper]

On 22/08/2012 13:13, goran...@gmail.com wrote:
> <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]

On Wednesday, August 22, 2012 10:07:48 PM UTC+3, Noah Roberts wrote:
> On Wednesday, August 22, 2012 11:32:26 AM UTC-7, Öö Tiib wrote:
> > On Wednesday, August 22, 2012 8:28:55 PM UTC+3, Noah Roberts wrote:
> > > 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 different ways.
> >
> > Yes. Refactoring inheritance into composition is the next step if needed.
>
> 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 is easy 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 of it 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 of all inheritance just by using composition always but in practice it is terribly expensive. One can have collection of member instead of one always if there 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...@gmail.com]

On Wednesday, August 22, 2012 6:52:12 PM UTC+2, Öö Tiib wrote:
> On Wednesday, August 22, 2012 3:13:33 PM UTC+3, (unknown) wrote:
>
> >
>
> >
>
> >
>
> > 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).
>
>
>
> 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.
>
>
>
> > 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.
>
>
>
> 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 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 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 see is "he took a stab on my RTTI". Well, now...

> > ( 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::open :-) )
>
>
>
> 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, 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.

Goran.

[Öö Tiib]

On Thursday, August 23, 2012 1:56:14 PM UTC+3, (unknown) wrote:
> On Wednesday, August 22, 2012 6:52:12 PM UTC+2, Öö Tiib wrote:
> > On Wednesday, August 22, 2012 3:13:33 PM UTC+3, (unknown) wrote:
> > > 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.
> >
> > 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 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 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 see is "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 suggesting a 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.

>
> > > ( 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::open :-) )
> >
> > 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?

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 you use 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).

> > 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, 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, where someone 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.

[Noah Roberts]

On Wednesday, August 22, 2012 4:23:53 PM UTC-7, Öö Tiib wrote:
> On Wednesday, August 22, 2012 10:07:48 PM UTC+3, Noah Roberts wrote:
>
> > 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 of it 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?

How so? You need to explain what you mean by "complexity". If you're talking order complexity I don't see how it's added unless it's actually needed. If you're talking about verbosity then I have two things to say:

1) verbosity is not necessarily a bad thing
2) it's quite simple and easy to write a reusable function that works for the generic, "unlock_door" functionality.

Furthermore, this is not "unneeded complexity" regardless of how you define complexity. It's needed to have a clean, extensible and robust design. This kind of design is always better than one that is not clean, extensible, or robust. It's intuitive in that it matches the real world fairly well, and there's nothing about it that would make it difficult for a programmer to use, extend, and maintain. All of these are more important than almost every other concern most of the time. There are times when you've really got to worry about performance or something like that, but you should profile before trying to second guess. I don't believe this design would show up on a profile as any kind of bottleneck as it doesn't add any order complexity when there's 1 or fewer locks.

One could easily argue for a composite lock solution that puts off adding the multiple lock interface to some time in the future when and if it's needed. It's a reasonable approach that just puts the "unlock_door" loop within a lock composite. Going this route one might be tempted to add a "null lock" that's never engaged. Lots of directions one could go from the point of deciding upon composition here.

Inheritance just seems like the wrong choice here that needlessly locks the future of the code down, provides no added benefit, and requires cross-casting, which should make any good designer question it...it's just not the right way to use interfaces.

[Öö Tiib]

On Thursday, August 23, 2012 6:41:38 PM UTC+3, Noah Roberts wrote:
> On Wednesday, August 22, 2012 4:23:53 PM UTC-7, Öö Tiib wrote:
> > On Wednesday, August 22, 2012 10:07:48 PM UTC+3, Noah Roberts wrote:
> > > 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 of it 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?
>
> How so? You need to explain what you mean by "complexity". If you're talking order complexity I don't see how it's added unless it's actually needed. If you're talking about verbosity then I have two things to say:
>
> 1) verbosity is not necessarily a bad thing
> 2) it's quite simple and easy to write a reusable function that works for the generic, "unlock_door" functionality.

I mean complexity of a relation in general. Lets let the doors rest for a second. If the relation is one_to_one then it is most simple to implement and most safe and efficient to use. One_to_maybe_one is slightly more complex relation. Unless it is clear compile time (if it is one or none) it adds some overhead to traverse it. One_to_any_number is more complex relation than previous. Anything traversing that relation has to support the whole complexity. Why all relations are not any_number_to_any_number? Because it is unneeded complexity.

> Furthermore, this is not "unneeded complexity" regardless of how you define complexity. It's needed to have a clean, extensible and robust design. This kind of design is always better than one that is not clean, extensible, or robust. It's intuitive in that it matches the real world fairly well, and there's nothing about it that would make it difficult for a programmer to use, extend, and maintain. All of these are more important than almost every other concern most of the time. There are times when you've really got to worry about performance or something like that, but you should profile before trying to second guess. I don't believe this design would show up on a profile as any kind of bottleneck as it doesn't add any order complexity when there's 1 or fewer locks.

Again, better let the doors and their problem domain rest. I brought it as an example just to avoid having abstract base class A's and base class B's. It typically happens that there is functionality of A that suddenly needs to take into account that descendents of it have gained functionality B that somehow interfere with that. The requirement is that A can not just fail but has to try to overcome it. Given RTTI example was how the situation is sometimes resolved with few lines of code. Nothing criminal there. Maybe try to pack up your words with code example?

I know that it can not be resolved elegantly by adding lists of members into A instead of its inheritance in general several classes have to be rewritten. Also I have seen the best case examples from books and mess that it produces in practice. It will not be efficient. Most flexible thing is probably "a graph of optional anythings", but it is never used. Why? Because it is unneeded complexity for any problem domain not clean and robust solution.

> One could easily argue for a composite lock solution that puts off adding the multiple lock interface to some time in the future when and if it's needed. It's a reasonable approach that just puts the "unlock_door" loop within a lock composite. Going this route one might be tempted to add a "null lock" that's never engaged. Lots of directions one could go from the point of deciding upon composition here.

One can go from simple code actually in lot more directions. Present unneeded one_to_any_number relation makes changing it more complex.

> Inheritance just seems like the wrong choice here that needlessly locks the future of the code down, provides no added benefit, and requires cross-casting, which should make any good designer question it...it's just not the right way to use interfaces.

How can it lock something down? All code is written like drawn on sand, nothing about it is like carved onto rock so nothing can lock any future down. Lot of code (big drawing) will make changing it more time consuming and raise unneeded difficulties and inefficiencies and that is it.

[Noah Roberts]

On Thursday, August 23, 2012 10:29:00 AM UTC-7, Öö Tiib wrote:
> Maybe try to pack up your words with code example?
>

struct lock {
virtual ~lock() {}
virtual void unlock() = 0;
virtual bool engaged() const = 0;
};

struct null_lock : lock {
void unlock() {}
bool engaged() const { return false; }
};

struct single_lock : lock {
void unlock() { state = UNLOCKED; }
bool engaged() const { return state == LOCKED; }
};

// can be added later...
struct multi_lock : lock {
// probably have this syntax wrong...
void unlock() { for ( lock & current_lock : locks) current_lock.unlock(); }

bool engaged() const {
return find_if(locks.begin(), locks.end(), bind(&lock::engaged, _1)) != locks.end();
}
iterator begin();
iterator end();
private:
?? locks;
};

struct door {
virtual ~door() {}
bool open() {
if (lock()->engaged()) return false;
state = OPEN;
return true;
}
lock* lock();

private:
lock my_lock;
door_state state;
};


Use code:

void go_through_door(door & entry) {
entry.lock()->unlock();
entry.open();
// go through...
}

Looks pretty darn simple to me. Compared to:

// may be a few lines of code smaller...but not much.
struct door { ... };
struct lockable { .... };
struct lockable_door : door, lockable { ... };

// but you pay for it here...
void go_through_door(door & entry) {
if (auto lock = dynamic_cast<lockable*>(&entry))
lock->unlock();
entry.open();
};

I contend that the first version is easier to read, easier to work with, results in simpler client code, does not fall prey to ANY design policy violations as it pertains to current requirements, and should be preferred under any condition I can conceive of. It's even possible that it would outperform the second, though I don't envision by much.

> How can it lock something down? All code is written like drawn on sand, nothing about it is like carved onto rock so nothing can lock any future down. Lot of code (big drawing) will make changing it more time consuming and raise unneeded difficulties and inefficiencies and that is it.

I follow SOLID principles in design. I'd suggest reading them and reading about why they result in better designs, particularly the open closed principle. Basically code very often DOES end up being carved into rock because the cost of change becomes too high. The more closely you can adhere to SOLID principles the less this will be the case.

The point isn't to get all worked up about it. The difference in this case is rather minor. The concept extends well beyond simple cases like this though and the advice to watch out for RTTI is reasonable. RTTI based code often violates some important design principle and in so doing will often result in code that is difficult to change and prone to error. RTTI based solutions can usually be replaced by better solutions that neither use RTTI nor violate design principles and they're often better in other respects as well. Like most things though this is not an absolute.

[Noah Roberts]

On Thursday, August 23, 2012 10:29:00 AM UTC-7, Öö Tiib wrote:
> On Thursday, August 23, 2012 6:41:38 PM UTC+3, Noah Roberts wrote:
>
> > On Wednesday, August 22, 2012 4:23:53 PM UTC-7, Öö Tiib wrote:
>
> > > On Wednesday, August 22, 2012 10:07:48 PM UTC+3, Noah Roberts wrote:
>
> > > > 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 of it 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?
>
> >
>
> > How so? You need to explain what you mean by "complexity". If you're talking order complexity I don't see how it's added unless it's actually needed. If you're talking about verbosity then I have two things to say:
>
> >
>
> > 1) verbosity is not necessarily a bad thing
>
> > 2) it's quite simple and easy to write a reusable function that works for the generic, "unlock_door" functionality.
>
>
>
> I mean complexity of a relation in general. Lets let the doors rest for a second. If the relation is one_to_one then it is most simple to implement and most safe and efficient to use. One_to_maybe_one is slightly more complex relation. Unless it is clear compile time (if it is one or none) it adds some overhead to traverse it. One_to_any_number is more complex relation than previous. Anything traversing that relation has to support the whole complexity. Why all relations are not any_number_to_any_number? Because it is unneeded complexity.

Your version is not a one_to_one BTW. What you've done is implement one_to_maybe_one by requiring the client code to attempt a cast to lockable for the check.

[Leigh Johnston]

On 23/08/2012 19:19, Noah Roberts wrote:

In general yes one should prefer composition over inheritance but the
proper way to do it using inheritance is really quite simple:

struct door : lockable
{
bool open()
{
if (!unlock())
return false;
}
virtual bool unlock()
{
/* ordinary doors have no locks */
return true;
}
};

struct external_door : door
{
virtual bool unlock()
{
/* external doors have a lock */
return lock.unlock();
}
private:
single_lock lock;
};

void go_through_door(door& entry)
{
if (!entry.open())
{
/* handle failure to open door */
}
}

/Leigh

[Öö Tiib]

Usually such cross casting is done by base class, not client code. Something like getLockable() of Door does the dynamic_cast. Also it sometimes is turned one_to_one for robustness of usage by returning pointer to some static dummy (like your null_lock) when dynamic_cast results with NULL pointer.

The outcome is roughly equal I see now. Some code is better than 1000 words. One has cross-cast other has a pointer member. More or less a tie for me in that problem domain. cross-cast has inherent flexibility when such "Lockable" needs to provide on its order some "Openable" when it becomes required that attempts to lock it may not fail with excuse that the door was open. Your design can overcome it with more pointers perhaps.

[Noah Roberts]

On Thursday, August 23, 2012 12:08:20 PM UTC-7, Leigh Johnston wrote:
> On 23/08/2012 19:19, Noah Roberts wrote:
>

As has already been discussed, having the base class contain an unlock function just so that you can call the behavior in a base class is not good design. You'll start to see the problem, perhaps, when you consider how you'd implement:

struct sliding_door : door { ... };

Once again, some will have locks and some won't. Going to have two classes for this too?? Tiib's runs into the same difficulty. It gets even worse when you start having many different kinds of locks and what you end up getting is subclass explosion and often this comes with a lot of redundant copy-pasta.

The recommended pattern to combat this sort of problem is the bridge. That's basically what the door->lock relation creates.

[Noah Roberts]

On Thursday, August 23, 2012 12:22:01 PM UTC-7, Öö Tiib wrote:

> The outcome is roughly equal I see now. Some code is better than 1000 words. One has cross-cast other has a pointer member. More or less a tie for me in that problem domain. cross-cast has inherent flexibility when such "Lockable" needs to provide on its order some "Openable" when it becomes required that attempts to lock it may not fail with excuse that the door was open. Your design can overcome it with more pointers perhaps.

Yeah, if you wanted to have the lock check the state of what it locks to make sure it can engage then you'd want a pointer back to the object it locks.

An alternative view would be that a lock can in fact engage with a door open, that door then just may not close. Some locks in fact can engage and allow you to shut the door. Car doors are like this. So you might have trouble implementing the former and it may depend entirely on the kind of lock (deadbolts keep the door from closing).

Funny story on this one...my cousin came to my house with her kid and he likes to lock doors. We left the house, closed the door, and talked as she was leaving. When I turned to go back into my home the door was locked and the keys were inside. I had to kick in my back door. When I bought the replacement I made sure to buy a knob that would not open from the inside while in the locked position.

This door->lock relationship could actually get quite complicated if we wanted to posit the various kinds of locks and doors. That's sort of the point I come from. Although you don't want to try to anticipate all of this crap when it's not needed, you also don't want to lock yourself from being able to implement it easily. A user could very well say, "Hey, that's neat...how about deadbolts or sliding doors?" This can be especially bad if they suddenly decide this 6 months down the road and you've got all this code betting on the wrong kind of interface. If you've used SOLID principles and thus avoided violating LSP and OCP you'll likely have an easier chance of doing this. Doesn't always work that way, since you can't be open to EVERYTHING, but it at least usually takes care of most of the obvious paths and many of the less obvious.

Yet another way to look at it is from the single responsibility principle. Doors provide ways to open and close entryways. Locks lock things. Keeping them distinct follows this principle as well while a "lockable door" is actually responsible for two things: locking and opening.

[Leigh Johnston]

On 23/08/2012 22:02, Noah Roberts wrote:
>
> As has already been discussed, having the base class contain an unlock function just so that you can call the behavior in a base class is not good design. You'll start to see the problem, perhaps, when you consider how you'd implement:
>
> struct sliding_door : door { ... };
>
> Once again, some will have locks and some won't. Going to have two classes for this too?? Tiib's runs into the same difficulty. It gets even worse when you start having many different kinds of locks and what you end up getting is subclass explosion and often this comes with a lot of redundant copy-pasta.

I agree that subclass explosion can happen if you go the inheritance
route however you have introduced the concept of locking into "door"
through composition even though a door can have no lock which means you
might as well derive "door" from "lockable".

If subclass explosion is starting to occur then you can refactor your
design from an inheritance based one into something using the strategy
and/or decorator patterns but if you only have a small number of "door"
types (in a game for example) the inheritance based solution might be
adequate. In addition to the "prefer composition to inheritance" rule
there is the rule "prefer static types to dynamic state".

/Leigh

[Öö Tiib]

On Friday, August 24, 2012 12:15:37 AM UTC+3, Noah Roberts wrote:
> On Thursday, August 23, 2012 12:22:01 PM UTC-7, Öö Tiib wrote:
> > The outcome is roughly equal I see now. Some code is better than 1000 words. One has cross-cast other has a pointer member. More or less a tie for me in that problem domain. cross-cast has inherent flexibility when such "Lockable" needs to provide on its order some "Openable" when it becomes required that attempts to lock it may not fail with excuse that the door was open. Your design can overcome it with more pointers perhaps.
>
> Yeah, if you wanted to have the lock check the state of what it locks to make sure it can engage then you'd want a pointer back to the object it locks.
>
> An alternative view would be that a lock can in fact engage with a door open, that door then just may not close. Some locks in fact can engage and allow you to shut the door. Car doors are like this. So you might have trouble implementing the former and it may depend entirely on the kind of lock (deadbolts keep the door from closing).

Yes the different abilities interfere with each other. So sometimes it is needed that the whole object orchestrates how the things really go. That is one reason why i hesitate when splitting ability as component.

> Funny story on this one...my cousin came to my house with her kid and he likes to lock doors. We left the house, closed the door, and talked as she was leaving. When I turned to go back into my home the door was locked and the keys were inside. I had to kick in my back door. When I bought the replacement I made sure to buy a knob that would not open from the inside while in the locked position.
>
> This door->lock relationship could actually get quite complicated if we wanted to posit the various kinds of locks and doors. That's sort of the point I come from. Although you don't want to try to anticipate all of this crap when it's not needed, you also don't want to lock yourself from being able to implement it easily. A user could very well say, "Hey, that's neat...how about deadbolts or sliding doors?" This can be especially bad if they suddenly decide this 6 months down the road and you've got all this code betting on the wrong kind of interface. If you've used SOLID principles and thus avoided violating LSP and OCP you'll likely have an easier chance of doing this. Doesn't always work that way, since you can't be open to EVERYTHING, but it at least usually takes care of most of the obvious paths and many of the less obvious.

Interface is basically same or takes 5 minutes to refactor into same. If the getLockable() returns the result of cross-cast or data member (or one or other depending on type when it is made virtual) makes no external diffrence.

> Yet another way to look at it is from the single responsibility principle. Doors provide ways to open and close entryways. Locks lock things. Keeping them distinct follows this principle as well while a "lockable door" is actually responsible for two things: locking and opening.

That is what i understand but the solution i am not buying. Imagine that i have 100 classes. I may have the abilities split out say there are 50 abilities in ability hierarchy. Each class combines 8 abilities in average. When i have rigid classes (a class inherits the abilities) then i have to test them 100 and that is it. When i have flexible (abilities are members) then ... doh i think it is something like 100 000 or more combinations that may make sense.

If i really only need the fixed 100 combos then making them flexible would just mean that my unit-tests run half a week pointlessly. That is second reason why i hesitate refactoring abilities into members without objective need.

Sure, in some domain i may need to go the flexible way and test all that pile of what is possible to make and filter maybe only utter nonsense regardless. For example when it is some framework i want to sell then 100 000 things is nice argument.

[Noah Roberts]

On Aug 23, 4:22 pm, Öö Tiib <oot...@hot.ee> wrote:

> That is what i understand but the solution i am not buying. Imagine that i have 100 classes. I may have the abilities split out say there are 50 abilities in ability hierarchy. Each class combines 8 abilities in average. When i have rigid classes (a class inherits the abilities) then i have to test them 100 and that is it. When i have flexible (abilities are members) then ... doh i think it is something like 100 000 or more combinations that may make sense.
>
> If i really only need the fixed 100 combos then making them flexible would just mean that  my unit-tests run half a week pointlessly. That is second reason why i hesitate refactoring abilities into members without objective need.

This isn't how to go about unit testing. Test the door and lock
separately. Use a mock/fake for the lock when you're testing the
door's functionality and if the lock needs to know about the door to
do its thing you do the same there. You don't need to run the full
combination of every possible assignment of lock and door. That's the
whole point of unit testing actually.

Let the next levels, such as integration and functional testing, pick
up anything that results from some specific combination. When and if
such a bug is found it will have something to do with one or both of
the units in question and it will point out something you missed in
your unit test...add it. It is quite normal for integration and
functional testing to take a while, which is another reason to keep
your unit testing small and fast...but complete for each unit.

I don't know why you're talking about refactoring here, I thought we
agreed the assumption was a new product design. I would not refactor
without some feature that can't be made the other way or was being
made more difficult, but I would not design it with inheritance from
the start for the reasons I've mentioned already...all of which ARE
objective reasons that I at least consider "needs" (such as future
maintainability).

Another issue with this conversation has been that you seem to be
taking a very simplistic, poorly thought out example to discuss
theoretical issues and used it specifically. While I've been
explaining ways in which to approach this example in terms that are
universal, you've been harping on this idea that you can make a
function to wrap up the cast. This is true here, but very often NOT
true with other, similar such constructs. Using inheritance as your
primary form of reuse is just plain wrong and leads to brittle,
unmaintainable code. Although this was how people thought OO would be
used when it first came out, it was realized fairly quickly that it
doesn't work that way. Inheritance should only be used for conceptual
organization--is-a relationships--and nothing else.

The problem with using a simplistic example to discuss such a far
reaching concept is that there can always be these, "but I don't need
it in this case," type responses. Unless we really dig into an actual
specification for this example problem we simply cannot know that.
Are we coding some game where doors are simple and they either lock or
don't? Maybe your solution is good enough (I wouldn't ever call it
the right one and would see it as massive overdesign for this
problem). Get much more complicated than that though and your
solution will begin to fall apart quite rapidly and you'll naturally
progress into a compositional relationship, which is simply preferable
to begin with because it follows well established design principles.

Anyway, someone somewhere asked how to do what they were doing without
RTTI and I've provided at least one good way now. Using RTTI isn't
necessarily bad, it's a smell that indicates something possibly bad
and/or that something could be done better. When you see RTTI being
used, look for violations of SOLID principles, which though are called
"OO Principles" extend quite readily into other paradigms like generic
programming.

[Öö Tiib]

On Sunday, August 26, 2012 9:03:59 PM UTC+3, Noah Roberts wrote:
> On Aug 23, 4:22 pm, Öö Tiib <oot...@hot.ee> wrote:
> > That is what i understand but the solution i am not buying. Imagine that i have 100 classes. I may have the abilities split out say there are 50 abilities in ability hierarchy. Each class combines 8 abilities in average. When i have rigid classes (a class inherits the abilities) then i have to test them 100 and that is it. When i have flexible (abilities are members) then ... doh i think it is something like 100 000 or more combinations that may make sense.
> >
> > If i really only need the fixed 100 combos then making them flexible would just mean that  my unit-tests run half a week pointlessly. That is second reason why i hesitate refactoring abilities into members without objective need.
>
> This isn't how to go about unit testing. Test the door and lock
> separately. Use a mock/fake for the lock when you're testing the
> door's functionality and if the lock needs to know about the door to
> do its thing you do the same there. You don't need to run the full
> combination of every possible assignment of lock and door. That's the
> whole point of unit testing actually.

Whole point of unit testing is to discover some of the defects earlier.
'Earlier' means before they get into repository and before they get into
build and before they start to cause more work to other people.
I am not sure why to split the tests at so low level? For example if design
say a template X<T> should not accept int as argument then why not to make
test that expects a static assert on X<int>?

> Let the next levels, such as integration and functional testing, pick
> up anything that results from some specific combination. When and if
> such a bug is found it will have something to do with one or both of
> the units in question and it will point out something you missed in
> your unit test...add it.

Call it 'integration test', 'regression test' or whatever, like we saw
earlier the different abilities affect each other so depending on the
tested situation such test may still need actual or mocking or fake or
dummy code. Since it ensures that code matches with design I call it
'unit test' as well, for simplicity.

If to bring parallel with doors then test 'knock-ability', 'lock-ability',
'pass-ability' and 'open-ability' of whole doors too. In project with some
history such tests contain majority of test code.

> It is quite normal for integration and
> functional testing to take a while, which is another reason to keep
> your unit testing small and fast...but complete for each unit.

Second level test for me after 'unit tests' is 'automatic tests', that
run on whole module and test it with real or generated scenarios.
Usually in environment of fake other modules. It is just to save the
human-testers time. May run constantly somewhere, i did not mean those.

> I don't know why you're talking about refactoring here, I thought we
> agreed the assumption was a new product design. I would not refactor
> without some feature that can't be made the other way or was being
> made more difficult, but I would not design it with inheritance from
> the start for the reasons I've mentioned already...all of which ARE
> objective reasons that I at least consider "needs" (such as future
> maintainability).

I am talking about refactoring in the context of your words: "This can

be especially bad if they suddenly decide this 6 months down the road
and you've got all this code betting on the wrong kind of interface."

It happens with all projects that things turn 90 or even 180 degrees with
lot smaller intervals than 6 months. If something is difficult to refactor
then that is major flaw. I am not sure what you mean by wrong kind of
interface, so was just saying that the interface is pretty compatible.

> Another issue with this conversation has been that you seem to be
> taking a very simplistic, poorly thought out example to discuss
> theoretical issues and used it specifically. While I've been
> explaining ways in which to approach this example in terms that are
> universal, you've been harping on this idea that you can make a
> function to wrap up the cast. This is true here, but very often NOT
> true with other, similar such constructs. Using inheritance as your
> primary form of reuse is just plain wrong and leads to brittle,
> unmaintainable code. Although this was how people thought OO would be
> used when it first came out, it was realized fairly quickly that it
> doesn't work that way. Inheritance should only be used for conceptual
> organization--is-a relationships--and nothing else.

Is there a case of that "very often NOT true" where some "interface
getter" can not use cross-cast internally? It feels equal to forwarding a
reference to a member or other aggregate. Sure, lockable door is-a
lockable, what is wrong using inheritance there? Or ... when you see a
door what is wrong about asking "are you lockable?"? If it is wrong in
sense of some general principle of OOD then too bad for OOD. We maybe need
better example to discuss that side?

There are tons of ways how to reuse and combine functionality in C++
into real class. Some assortment of ways (far not all of them):

0) You do not need that functionality. Do not reuse it in any way. Lack
of any code is most simple.
1) Use template. Good template is hard thing to make and to test
sufficiently, but it is extremely efficient and reusable.
2) Forward functionality of non-public base class. If there are no
polymorphism then it is very efficient. Easier than (3) to use, bit
harder to understand.
3) Forward functionality to non-polymorphic data member. When the data
member is template then it is as efficient as (2) otherwise it carries
very little overhead. Easiest to understand.
4) Static polymorphism (CRTP). Very little overhead if any. Adds some
flexibility of (5), but is hardest to understand and to make since
it involves (1) with properties of (5).
5) Usual dynamic polymorphism. With its virtual functions it is bit less
efficient than others above. Somewhat overused, over-taught and
under-understood.
6) Forward functionality to polymorphic data member. That is both most
complex and flexible since polymorphic data member means that it may
change during object's life-time. More overhead than with (5).

Can not be that the SOLID OOD suggests to always use that (6) as very
first choice of functionality re-usage ... it seems least efficient and
not most simple.

> The problem with using a simplistic example to discuss such a far
> reaching concept is that there can always be these, "but I don't need
> it in this case," type responses.

Ok, probably my fault. I did not want to base all considerations of
re-usage of functionality on one simplistic example.

> Are we coding some game where doors are simple and they either lock or
> don't?

No. I have explained else-thread. It was just used to avoid typical
class A, class B blah-blah too abstract example. I did not want to
introduce some problem with doors. I wanted to bring example of the usage
of cross cast.
As for doors ... yes let them rest ... Possibly the usage of vector of
polymorphic locks is the perfect choice for a door in some game-world.
It may also be that it is overkill and just a data member with fitting
key # is needed in some other game-world. That is why i have said that you
can let the doors to rest.

> Anyway, someone somewhere asked how to do what they were doing without
> RTTI and I've provided at least one good way now. Using RTTI isn't
> necessarily bad, it's a smell that indicates something possibly bad
> and/or that something could be done better. When you see RTTI being
> used, look for violations of SOLID principles, which though are called
> "OO Principles" extend quite readily into other paradigms like generic
> programming.

Yes it was me. It was good discussion.
Your suggestion to go to next level and turn inheritance into component
can be correct choice. What i am still discussing is that you seem to
suggest to always design inheritance as component. I have seen it suggested elsewhere and i do not see it at all as profitable on general case.

The way suggested by Scott and Leigh to combine all cases (to make all Doors
Lockable) was maybe most robust. It turns it into usual polymorphic usage.

The pre_opener of Goran (maybe also your, you mentioned visitor?) was to use
some double-dispatch to get rid of dynamic-cast. That raises complexity and
often feels like wheel-chair. It can work best with legacy code that should
not be modified.

Were there others? Sorry if i missed.

I myself still think that unless it burns (shows signals of under- or
over-engineering) somewhere else ... then that cross-cast there is fine. ;)

[Noah Roberts]

On Sunday, August 26, 2012 6:42:11 PM UTC-7, Öö Tiib wrote:
> On Sunday, August 26, 2012 9:03:59 PM UTC+3, Noah Roberts wrote:
>
> > On Aug 23, 4:22 pm, Öö Tiib <oot...@hot.ee> wrote:
>
> > > That is what i understand but the solution i am not buying. Imagine that i have 100 classes. I may have the abilities split out say there are 50 abilities in ability hierarchy. Each class combines 8 abilities in average. When i have rigid classes (a class inherits the abilities) then i have to test them 100 and that is it. When i have flexible (abilities are members) then ... doh i think it is something like 100 000 or more combinations that may make sense.
>
> > >
>
> > > If i really only need the fixed 100 combos then making them flexible would just mean that  my unit-tests run half a week pointlessly. That is second reason why i hesitate refactoring abilities into members without objective need.
>
> >
>
> > This isn't how to go about unit testing. Test the door and lock
>
> > separately. Use a mock/fake for the lock when you're testing the
>
> > door's functionality and if the lock needs to know about the door to
>
> > do its thing you do the same there. You don't need to run the full
>
> > combination of every possible assignment of lock and door. That's the
>
> > whole point of unit testing actually.
>
>
>
> Whole point of unit testing is to discover some of the defects earlier.
>
> 'Earlier' means before they get into repository and before they get into
>
> build and before they start to cause more work to other people.
>
> I am not sure why to split the tests at so low level? For example if design
>
> say a template X<T> should not accept int as argument then why not to make
>
> test that expects a static assert on X<int>?

You can. You brought up a problem saying you'd get this massive explosion having to test every combination. I was explaining why this is not necessary. Sometimes it is in fact easier to just test with the real client objects than to build mocks.

We could argue about what's the point of unit tests forever I suppose. In my mind it is to make refactoring simpler and to be able to test individual components by themselves, thus making it easier to work on them. Testing whether the program does what it's supposed to do without crashing is QA's job.

>
> Call it 'integration test', 'regression test' or whatever, like we saw
>
> earlier the different abilities affect each other so depending on the
>
> tested situation such test may still need actual or mocking or fake or
>
> dummy code. Since it ensures that code matches with design I call it
>
> 'unit test' as well, for simplicity.

Having a hard time figuring out what you mean by "ability". You can change what terms mean and call whatever you want "unit test", but then it's pretty damn tough to figure out what you're saying.

>
> Second level test for me after 'unit tests' is 'automatic tests', that
>
> run on whole module and test it with real or generated scenarios.
>
> Usually in environment of fake other modules. It is just to save the
>
> human-testers time. May run constantly somewhere, i did not mean those.

Unit tests should be automated, so I'm again having trouble figuring out what you mean.

> I am talking about refactoring in the context of your words: "This can
>
> be especially bad if they suddenly decide this 6 months down the road
>
> and you've got all this code betting on the wrong kind of interface."

My point was to design it the right way to begin with.

> It happens with all projects that things turn 90 or even 180 degrees with
>
> lot smaller intervals than 6 months. If something is difficult to refactor
>
> then that is major flaw. I am not sure what you mean by wrong kind of
>
> interface, so was just saying that the interface is pretty compatible.

The wrong interface is one that violates basic principles of OO design for no good reason besides, "I like inheritance better than composition."

> There are tons of ways how to reuse and combine functionality in C++
>
> into real class. Some assortment of ways (far not all of them):
>
>
>
> 0) You do not need that functionality. Do not reuse it in any way. Lack
>
> of any code is most simple.
>
> 1) Use template. Good template is hard thing to make and to test
>
> sufficiently, but it is extremely efficient and reusable.
>
> 2) Forward functionality of non-public base class. If there are no
>
> polymorphism then it is very efficient. Easier than (3) to use, bit
>
> harder to understand.
>
> 3) Forward functionality to non-polymorphic data member. When the data
>
> member is template then it is as efficient as (2) otherwise it carries
>
> very little overhead. Easiest to understand.
>
> 4) Static polymorphism (CRTP). Very little overhead if any. Adds some
>
> flexibility of (5), but is hardest to understand and to make since
>
> it involves (1) with properties of (5).
>
> 5) Usual dynamic polymorphism. With its virtual functions it is bit less
>
> efficient than others above. Somewhat overused, over-taught and
>
> under-understood.
>
> 6) Forward functionality to polymorphic data member. That is both most
>
> complex and flexible since polymorphic data member means that it may
>
> change during object's life-time. More overhead than with (5).
>
>
>
> Can not be that the SOLID OOD suggests to always use that (6) as very
>
> first choice of functionality re-usage ... it seems least efficient and
>
> not most simple.

I certainly said no such thing. It's pretty hard to have a conversation with someone that's going to straw man everything I say.

You can use templates for composition. You can use inheritance correctly. You can use external functions rather than members or friends. All of these things adhere to SOLID principles quite nicely.

The template solution doesn't appear to work in this case because what is desired is a RUNTIME check for the difference between a door with a lock and one that has none. Anything based on static information then is probably going to have similar flaws.

> No. I have explained else-thread. It was just used to avoid typical
>
> class A, class B blah-blah too abstract example. I did not want to
>
> introduce some problem with doors. I wanted to bring example of the usage
>
> of cross cast.

It's a decent example for that and shows exactly the kinds of flaws that come about from that kind of choice.

> > Anyway, someone somewhere asked how to do what they were doing without
>
> > RTTI and I've provided at least one good way now. Using RTTI isn't
>
> > necessarily bad, it's a smell that indicates something possibly bad
>
> > and/or that something could be done better. When you see RTTI being
>
> > used, look for violations of SOLID principles, which though are called
>
> > "OO Principles" extend quite readily into other paradigms like generic
>
> > programming.
>
>
>
> Yes it was me. It was good discussion.
>
> Your suggestion to go to next level and turn inheritance into component
>
> can be correct choice. What i am still discussing is that you seem to
>
> suggest to always design inheritance as component.

I certainly never said any such thing.

> The way suggested by Scott and Leigh to combine all cases (to make all Doors
>
> Lockable) was maybe most robust. It turns it into usual polymorphic usage.

This is only correct if you're not adding functions to your base only for some of its concrete inheritors. If it makes sense to ask a door that has no lock to unlock then this is an appropriate solution. It makes no sense in the real world, but that does not mean it doesn't in code. Scott attempted to add the function just because one type of door had use for it and this is the wrong reason (reasons matter). Leigh on the other hand suggested that my solution was no different and he's probably right...at this point it begins to make more sense to simplify the algebra.

> I myself still think that unless it burns (shows signals of under- or
>
> over-engineering) somewhere else ... then that cross-cast there is fine. ;)

It already does show those signs. Cross casting is a smell and can violate both LSP and OCP at least.