Making this code properly polymorphic

Question

I have an abstract class Parent, which has multiple children and blank functions for interacting with each of these children. Each Child overrides Parent's functions and interacts with other Childs in different ways; i.e. Child1 has different implementations for interact_with(Child1), interact_with(Child2), etc etc.

In Parent, I have a function interact_with(Parent foo). Every Child looking to interact with another Child must pass through this function first. Until now everything is good, but then we run into a problem: after some basic logic has been processed, the Child then needs to know the specific type of its parameter so it can go on and call its own overridden function. At the moment I have this:

Child1* child1 = dynamic_cast<Child1*>(foo);
Child2* child2 = dynamic_cast<Child2*>(foo);
Child3* child3 = dynamic_cast<Child3*>(foo);

if(child1 != nullptr){
    interact_with(child1)
}

else if(child2 != nullptr){
    interact_with(child2)
}

else if(child3 != nullptr){
    interact_with(child3)
}

It works, but it isn't a very good solution. It gets especially bad when I have so many classes. Is this indicative of flawed base design, and if so, how would I improve this?

EDIT: To clarify: I have something like this

//Parent is an abstract class
class Parent
{
    void interact_with(Parent* foo){
        //this is here because there is a lengthy code segment
        //that needs to be run no matter what child interacts
        //with which

        //afterwards, I need to interact with whatever foo really is
    }

    virtual void interact_with(Child1* child){*blank*};
    virtual void interact_with(Child2* child){*blank*};
    virtual void interact_with(Child3) child){*blank*};
};

class Child1 : public Parent
{
    virtual void interact_with(Child1* child){*do something*};
    virtual void interact_with(Child2* child){*do something else*};
    virtual void interact_with(Child3* child){*do something else*};
};

Already.

Answer 1

_{Warning: this solution will break your interfaces. But don't worry, they are already broken ;)}

In my opinion your design mistake is following: although all children are "equal" you pick one of them to be responsible for interactions and call a method on it (And if you want 3, 4, ..., N equal children (in an array) to interact simultaneously, which one is responsible?)

If in your application all objects are equally important and no object is responsible for interactions, you should move interactions into free overloaded binary functions:

void interact(Child1* a, Child1* b);

void interact(Child1* a, Child2* b);

...

void interact(Child2* a, Child1* b)
{
    interact(b, a); // if order does not matter, reuse another function 
}

Clearly, it won't solve the problem of boilerplate code, but at least it could help you to re-think your design and to find a better solution than double dispatch or casting.

Also, depending on functions internals, you could probably reduce writing (but not code size) easily by using template functions instead of overloaded ones.

Answer 2

The @imreal answer using double dispatch is correct. However, the dispatches can be done using virtual member functions instead of a map and function pointers (which is actually similar to the vtable the compiler generates).

The problem is that a single virtual function will not solve the problem, because you really need a double dispatch (i.e. a virtual call regarding both objects, not just the one being called).

See the following working example:

#include <iostream>

class Child1;
class Child2;

class Parent
{
public:
    virtual void interact_with(Parent* other) = 0;
    virtual void interact_with(Child1* child) {};
    virtual void interact_with(Child2* child) {};
};

class Child1 : public Parent
{
public:
    virtual void interact_with(Parent* other)
    {
        other->interact_with(this);
    }
    virtual void interact_with(Child1* child)
    {
        std::cout << "Child1 - Child1\n";
    }
    virtual void interact_with(Child2* child)
    {
        std::cout << "Child1 - Child2\n";
    }
};

class Child2 : public Parent
{
public:
    virtual void interact_with(Parent* other)
    {
        other->interact_with(this);
    }
    virtual void interact_with(Child1* child)
    {
        std::cout << "Child2 - Child1\n";
    }
    virtual void interact_with(Child2* child)
    {
        std::cout << "Child2 - Child2\n";
    }
};

int main()
{
    Child1 c1;
    Parent* p1 = &c1; // upcast to parent, from p1, we don't know the child type
    Child2 c2;
    Parent* p2 = &c2;

    c1.interact_with(&c2); // single virtual call to Child1 - Child2
    p1->interact_with(&c2); // single virtual call to Child1 - Child2
    p1->interact_with(p2); // double virtual call to Child2 - Child1 (NB: reversed interaction)
}

It outputs:

Child1 - Child2
Child1 - Child2
Child2 - Child1

Note the last one is reversed. That's because to make the dynamic dispatch using virtual functions on the argument, I have to swap the this pointer with the argument. This is fine if these interactions are symmetric. If they aren't, then I'd suggest to create a wrapper around the most generic one swapping the this and the argument again.

Answer 3

Using dynamic_cast<> like that is bad design like you said. You should make the function interact_with virtual like this in the declaration.

virtual void interact_with(Parent foo);

This will make the method call use the subclass's implementation of interact_with instead of the parent class's. You can then replace everything you've written with just this.

interact_with(foo);

Here is a pretty good explanation of virtual methods.

Answer 4

What you want is double dispatch

There are many approaches that you might take depending on your requirements. A very generic one is having a simple map of the form Key(type, type) -> Value(function) that associates an argument type pair with the function to be called.

In this case you will need a set of free functions of the type void function(Parent*, Parent*) (one for every combination you need) and a map of the type std::unordered_map<std::pair<TypeId, TypeId>, FunctionType> where TypeId is some form of type identifier with value semantics.

Then you do the dispatch at runtime:

if(map_.find(make_pair(type1, type2)) != map_.end())
    map_[make_pair(type1, type2)](obj1, obj2);

Not before registering each function:

map_[make_pair(type1, type2)] = func12;
map_[make_pair(type2, type3)] = func23;
....