OO Design and avoiding dynamic casts

Started by
22 comments, last by Xai 11 years, 7 months ago
I'm pretty new to this stuff too but here are a few ideas:

Referring to your second example:

1. You don't need to know what type of class the IControl object is, just what family of classes it belongs to - KBM or Touch. Therefore you could:
* add two more interfaces called IKBMControl and ITouchControl, both of which extend from IControl, you only need to do a single cast (IControl -> IKBMControl / IControl -> ITouchControl) then.
* you could add an additional method to IControl called getFamily() which will return if it's a KBM or a Touch class.

2. The Abstract Factory pattern will only work if IControl completely captures the dependency between IControl and its clients KBMWindow and TouchWindow. However as KBMWindow and TouchWindow need to know the type behind the IControl interface, it would seem the interface is not sufficient.

If KBMWindow can only take KBM controls then its interface should probably reflect that, and the same for TouchWindow. This would mean that they wouldn't be able to extend from Window though. I.e.

KBMIWindow
+ addChildControl(IKBMControl) : void

TouchIWindow
+ addChildControl(ITouchControl) : void

IKBMControl and ITouchControl could extend from the common interface IControl if there are other clients which require funcitonality which can be used without knowing if they are KBMControl or TouchControl objects.

3. Another thought would be that you seem to have two axis of change here,
1. Control type, i.e. Button, Combo Box, Window
2. GUI framework

So in some situations the Bridge pattern may be useful. It would depend on how easy it is to abstract the GUI framework.

Just a few thoughts. It's an interesting question and it'll be good to see how other people would approach your problem.

Matt
Advertisement
The issue is that you are violating the Liskov Substitution Principle (LSP).
So your interface has set up this contract stating that IWindow::addControl takes an IControl. In reality this is a lie because concrete subclasses such as FooWindow in fact require a FooControl, not an IControl.

The LSP only permits contravariance of function parameter types in subclasses (which means making the type more general), whereas what you're achieving through the use of casting is covariance (making the type more specific). Failure to abide by the LSP will mean that (without casting and therefore also violating the Open-Closed Principle) you cannot safely use the objects polymorphically.

One solution is to conclude that addControl is not something you can abstract away, since a FooWindow needs FooControls and a QtWindow needs QtControls, so you simply have a non-virtual addControl(FooControl &) function in a FooWindow. If you want to do the GUI building in an API-agnostic fashion then use static duck-type polymorphism:

template <class FactoryT>
void buildGui(FactoryT guiFactory)
{
FactoryT::WindowType window = guiFactory.createWindow();
FactoryT::ControlType ctrlA = guiFactory.createControl();
FactoryT::ControlType ctrlB = guiFactory.createControl();

window.addControl(ctrlA);
ctrlA.addControl(ctrlB);
}


It turns out we actually can abstract addControl, we just have to realise that making it consume a Control as a parameter is futile. It will instead need to be a producer of controls, this works because the LSP does permit covariance of return types! So we can hide the implementation behind the return type.

class FooWindow : public IWindow
{
FooFramework::ApplicationWindow * theActualWindow;

IButton * addButton() {
FooButton * button = new FooButton();
theActualWindow->add( button->getFooFrameworkPtr() );
return button;
}
}

// elsewhere...
IWindow * window = new FooWindow();
IButton * button = window->addButton();


Another way to go is to just select the implementation at build-time. So you have a non-abstract Window class declaration in a header and several different source/cpp files that implement Window but for different frameworks (Foo, Qt, etc). Which cpp file you use is selected by the pre-processer using macros (or some other kind of variant on this idea.. linking a particular static library, etc). In order for Window to get access to the underlying framework poiner from a Control object the Control class can have a getUnderylingPtr() function, this could return a void pointer (and accept a static_cast, which we know will be safe, and fast, because the implementation was chosen statically), or abstract the type with a typedef that the implementation chooses, or use the curiously-recurring template pattern (CRTP) to avoid losing type information.
Given your current constraints, what I would do is make each control responsible for adding itself.

i.e.:


class Control
{
public:
virtual void addToWindow( KBMWindow& window )
{
throw std::logic_error("This control type not implemented for KBM.");
}
virtual void addToWindow( TouchWindow& window )
{
throw std::logic_error("This control type not implemented for Touch.");
}
};

class Button: public Control
{
public:
virtual void addToWindow( KBMWindow& window )
{
window.uiWindow().addControl( getKBMButton() );
}
virtual void addToWindow( TouchWindow& window )
{
window.uiWindow().addControl( getTouchButton() );
}
};

class KBMWindow: public Window
{
KBM::Window m_window;
public:
void addControl(Control *ctrl)
{
ctrl->addToWindow(m_window);
}

KBM::Window& uiWindow(){ return m_window; }
};

class TouchWindow: public Window
{
Touch::Window m_window;
public:
void addControl(Control *ctrl)
{
ctrl->addToWindow(m_window);
}

Touch::Window& uiWindow(){ return m_window; }
};

1. You don't need to know what type of class the IControl object is, just what family of classes it belongs to - KBM or Touch. Therefore you could:
* add two more interfaces called IKBMControl and ITouchControl, both of which extend from IControl, you only need to do a single cast (IControl -> IKBMControl / IControl -> ITouchControl) then.
* you could add an additional method to IControl called getFamily() which will return if it's a KBM or a Touch class.

* You're still left with a cast.
* Yes, creating your own RTTI system with ids


The issue is that you are violating the Liskov Substitution Principle (LSP).
So your interface has set up this contract stating that IWindow::addControl takes an IControl. In reality this is a lie because concrete subclasses such as FooWindow in fact require a FooControl, not an IControl.

The LSP only permits contravariance of function parameter types in subclasses (which means making the type more general), whereas what you're achieving through the use of casting is covariance (making the type more specific). Failure to abide by the LSP will mean that (without casting and therefore also violating the Open-Closed Principle) you cannot safely use the objects polymorphically.

This is the correct answer.


Given your current constraints, what I would do is make each control responsible for adding itself.

No. You would be adding a dependency from every concrete control to every concrete window. Also these "not implemented" exceptions are awful in public interface.

Just one more thought, real quick...

Another place this dynamic_cast issue comes up in a lot is with publish/subscribe messaging architectures. Often, you have an abstract interface called Message, and then any time you want to send data from one part of your program to another (where nothing knows at compile time who is communicating with whom), then you just subclass Message (e.g., FooMessage, BarMessage). However, the message handler method on the receiving end will get a Message& and it needs to down-cast that to the right message type before it can deal with it.


You can automate this with templates (not tested, but the concept works):

class message_dispatcher
{
private:
struct abstract_listener
{
void* listener;
void (*handle)( void* listener , const void* message );
};
template< class MessageType , class ReceivingType >
static void handle_func( void* listener , const void* message )
{
ReceivingType& typed_listener = *static_cast< ReceivingType* >( listener );
const MessageType& typed_message = *static_cast< const MessageType* >( message );
typed_listener.handle( typed_message );
}

std::multimap< type_info , abstract_listener > listeners;

public:
template< class MessageType , class ReceivingType >
void subscribe( ReceivingType& r )
{
abstract_listener al{ &r , &handle_func< MessageType , ReceivingType > };
type_info info = typeid( MessageType );

listeners.insert( { inf , al } );
}
template< class MessageType >
void dispatch( const MessageType& msg )
{
type_info info = typeid( MessageType );
auto range = listeners.equal_range( info );

while( range.first != range.second )
{
( *(range.first->handle_func) )( range.first->listener , &msg );
++range.first;
}
}
};
struct message_a
{};
class specific_listener
{
public:
void handle( message_a& m )
{

}
};

specific_listener listener;
message_dispatcher dispatcher;
dispatcher.subscribe< message_a >( listener );
dispatcher.dispatch( message_a() );


Notice, however, that you can't use a message hierarchy. If you dispatch a B : A, a handler that has subscribed for A won't get that message.
If you need the performance, you can change std::multimap to something like a hash map and replace type_info with your own ids, e.g.

template< class T >
void* t_id()
{
static char c;
return c;
}
typedef void* msg_id;
msg_id id = t_id< message_a >();

//or if you need to communicate between different dlls

struct message_a
{
static const unsigned int id = //some unique value
};

The issue is that you are violating the Liskov Substitution Principle (LSP).

This is the most correct answer (I did not quote more because it is not necessary).

I have never used a dynamic cast in my life, over probably 100 projects, big and small.

If you are ever using dynamic_cast, you are doing it wrong, either in principle or in implementation. In this case it is on principle.
There is always a better solution to dynamic_cast.


L. Spiro

I restore Nintendo 64 video-game OST’s into HD! https://www.youtube.com/channel/UCCtX_wedtZ5BoyQBXEhnVZw/playlists?view=1&sort=lad&flow=grid


YAGNI

There is invariably enough subtle changes in behavior for this sort of thing that you're going to end up doing a lot of re-write/re-work anyways. Just use what works.


This is the pragmatically correct answer to the OP. The only way you could possibly get this kind generic-ness into a meta-GUI framework and have it actually work for a variety of concrete frameworks is by knowing the thousands of details required to be taken care of for the set of GUI frameworks that you want to wrap. You would need to know ahead of time that you want to wrap Qt, wxWidgets, Foo32, ... e.g. oh, Foo32 doesn't let buttons contain child objects but all the other ones do and I expect button labels to be normal label objects that have buttons as parents, etc. etc. etc. etc. times a million.
Thanks, guys. I'm not ignoring your responses, I am just processing them. I'll have some more questions/thoughts later.

The main point is, I may have more than one concrete implementation of the abstract factory/product set. So, let's say I have:


This post you made (first half) nails the whole idea between abstract factory correctly. Abstract factory is specifically for this exact special case, where there are multiple sets of classes that work with each other, and only each other, but other families of classes that have the same pattern. Such as if you had the touch / keyboard interface you describe, or my standard answer, the OpenGL and DirectX widget implementations. Obviously for these types of situations the OUTSIDE user wants to use JUST the interfaces (in other words the GAME doesn't care about OpenGL vs DirectX, so it uses the IControl, and IWindows interfaces EXCLUSIVELY). However of course, as you are aware the classes themselves, internally, DO care. An OpenGL control only consumes OpenGL textures and draws in OpenGL windows. So the key here is that the INTERNAL logic is more complex than the EXTERNAL logic.

Understand that Abstract Factory isn't telling you about how to make the OpenGL family talk to each other, its telling you how to make it so the outside world instantiates a factory once (and only once) and uses that ONE factory to create all of its widgets, so that it can be completely ignorant of what type of widget they are. Kinda like saying, as long as you are in a Verizon store, any phones you buy will work on the Verizon network. So once you decided which store to walk into, you can remain ignorant of the compatibility issue.

So, will you need down-casts for this case ... probably (although there are some techniques and languages that do not require it). However realize that your example of such a need is very flawed.

An OpenGL window should not be running around casting OpenGL controls to a long list of all possible children types, that is just madness. That is an unmanageable, unextendable non-oo design. However each OpenGL control can and likely will cast the IWindow to an IOpenGL window (fully expecting it to be one, since the whole assumption is the creation of everything from exactly 1 factory instance, so that a mismatch would be impossible. Also, an OpenGL window would keep a list of IOpenGL controls, and cast each control on add from IControl to IOpenGL control ... because the open gl family is going to be implemented internally to receive, associate with and use only other open gl family controls. The base interfaces are just used as the lesser API for externally facing interfaces.

So seeing the logical equivalent of:

List<IGLControl*> controls;
controls.add(dynamic_cast<IGLControl*>(control);

is totally normal for this type pattern.

But seeing

dynamic_cast<IGLComboBox*>(control);

would NOT be normal. The difference? In the first case you are downcasting from some interface, to its FAMILY SPECIFIC SPECIALIZATION (of which there can only be 1), which has the exact same PURPOSE within the family as the base interface, but just adds necessary family specific details to allow proper operation. In the second case you are trying to downcast from some general interface to another interface, which has a more specialized purpose - of what use is a window having a list of control, if the window has to know what type they are to use them ... this is the opposite of polymorphic

Another example - if would be like - instead of letting a class (Teacher) walk through a list of children (Students) asking them to perform the same operation (AreYouReadyForTheFeildTrip?) - which each student determine using different methods. Your flawed example would be like the teacher deciding what to ask the student based on first identifying who they are (Tommy or Jane?) and then if Tommy, doing the Tommy specific code, and if Jane doing the Jane specific code. When the whole point of polymorphic is to let Tommy, Jane and Ivan's creator (programmer / parents) make each one unique as they should be - but their Teacher no longer has to understand these differences to interact with them.

Another example - if would be like - instead of letting a class (Teacher) walk through a list of children (Students) asking them to perform the same operation (AreYouReadyForTheFeildTrip?) - which each student determine using different methods. Your flawed example would be like the teacher deciding what to ask the student based on first identifying who they are (Tommy or Jane?) and then if Tommy, doing the Tommy specific code, and if Jane doing the Jane specific code. When the whole point of polymorphic is to let Tommy, Jane and Ivan's creator (programmer / parents) make each one unique as they should be - but their Teacher no longer has to understand these differences to interact with them.


I like your example, it makes a lot of sense. I have run into a similar problem in another context. Yet, I am still not sure what the best solution might be. The difficulties arose when I was thinking about how to design a shader system that would allow a lot of reuse and flexibility. Let us consider the following simplified case with the following elements/classes:

Effect - The compiled and linked shader program. It expects a certain set of inputs that have to be bound in order to execute properly. This is kind of important: The requirements in terms of inputs are dictated by the effect.
Material - Contains a set of uniforms that define the appearance of an object. In order to render an object, the material is bound to the effect.
Geometry - Contains attribute buffers that describe the geometry of the object. In order to render an object, the geometry is bound to the effect.
Object - Contains a Material and a Geometry (possibly also an Effect, depends on whether we want to be able to specify different effects for different objects). Can be rendered in some way (Renderer::render(Object object) or something like that).

So far I believe this is fairly standard. Now, in OOP an obvious approach would be to define generic base classes and subclass them. But now, I have exactly the two options that were described, but I am not happy with either one. Let me explain (and I will just focus on the material, because the geometry is more or less analogous):

First Option:
The base classes define an interface like this:

[source]Effect
+ void Load(string file) // load / compile / link
+ void Bind(Material material)

Material
* Empty *[/source]

And subclasses might look like this:

[source]ColorEffect
- int AmbientLocation
- int DiffuseLocation
+ void Load(string file) // base.load and ensure the program takes the required inputs, get AmbientLocation, DiffuseLocation
+ void Bind(Material material) // bind the uniforms to the appropriate locations

ColorMaterial
+ vec3 Ambient
+ vec3 Diffuse[/source]

Now, this design that would be considered wrong, I suppose. It requires exactly the kind of dynamic_cast that has been criticized. The Effect::Bind(Material material) method would contain something like this (Pseudocode):

[source]ColorMaterial colorMaterial = dynamic_cast<ColorMaterial>(material);

glUniform(AmbientLocation, colorMaterial.AmbientColor);
glUniform(DiffuseLocation, colorMaterial.DiffuseColor);[/source]

What is kind of neat about this is that the Effect knows exactly what it needs and takes it from the material. You could pass a more derived material and it would still work. Even less derived materials could work, however they would need special handling and the effect would provide default values for the missing uniforms.

Second Option:
Base classes look like this:

[source]Effect
+ void Load(string file) // load / compile / link

Material
+ void BindTo(Effect effect)[/source]

And subclasses look like this:

[source]ColorEffect
+ int AmbientLocation
+ int DiffuseLocation
+ void Load(string file) // load / compile / link

ColorMaterial
+ vec3 Ambient
+ vec3 Diffuse
+ void BindTo(Effect effect)[/source]

At first glance, this may look better. However, it still requires the dynamic_cast to get the concrete ColorEffect in the BindTo method. One could argue that it is an improvement to do the binding code in the material, because the effect no longer has to differentiate between different materials. But you lose some of the good properties of the first variant. In order to make more derived materials bind to less derived effects, you have to introduce special cases in the BindTo method.

So, yeah, this is probably a flawed design, but I find it difficult to come up with a better solution without sacrificing flexibility. Maybe my mistake is that I am trying to be too generic (base classes are empty/almost empty). Essentially, it would be nice to have a system where you can change the effect without changing the material, and everything still works as long as the material provides the necessary inputs to the effect.

I think this post is getting pretty long, I'm curious to hear your thoughts.

This topic is closed to new replies.

Advertisement