Casting base to derived class according to a type flag

Question

I have been writing an event class for my game engine and I came across to the following problem:

Is casting a base class object to a derived class object given a type flag a good programming design?

Let me give you the following simple example. I have this enum holding the categories of the events:

enum DisplayEventType
{
    DET_UNSET,
    DET_WINDOW_EVENT,
    DET_KEYBOARD_EVENT,
    DET_MOUSE_EVENT,
    DET_JOYSTICK_EVENT
};

This is the base class of all the events:

typedef std::unique_ptr<DisplayEventData> DisplayEventDataMP;

class DisplayEvent
{
    private:

        // The type of the display event
        DisplayEventType mType;

        // The data of the event
        DisplayEventDataMP mEventData;

    public:

        // Constructor
        DisplayEvent(DisplayEventType type = DisplayEventType::DET_UNSET, DisplayEventDataMP eventData = std::unique_ptr<DisplayEventData>((DisplayEventData*) 0));

        // Retrieves the event type of the current event
        DisplayEventType GetType() const;

        // Sets the event type of the current event
        void SetType(DisplayEventType type);

        // Retrieves a raw pointer to the event data of the current event
        DisplayEventData* GetEventData() const;

        // Sets the event data of the current event
        void SetEventData(DisplayEventDataMP eventData);
};

This is the base class of the event data:

class DisplayEventData
{
    public:

        // Virtual destructor, for the deallocation of the interface implementors to work
        virtual ~DisplayEventData();
};

And these are the derived classes

class WindowEventData : public DisplayEventData
{
    // Window event data specific data and methods
};

class KeyboardEventData : public DisplayEventData
{
    // Keyboard event data specific data and methods
};

class MouseEventData : public DisplayEventData
{
    // Mouse event data specific data and methods
};

class JoystickEventData : public DisplayEventData
{
    // Joystick event specific data and methods
};

Some basic usage would be this:

// Poll for next event in the queue
DisplayEvent ev;
display->PollEvent(ev); // Fills ev with the needed data

if (ev.GetType() == DisplayEventType::DET_KEYBOARD_EVENT)
{
    KeyboardEventData* kev = static_cast<KeyboardEventData>(ev.GetEventData());
    // Do stuff
}

Is the above design viable/correct? If not what would a better alternative be in terms of object oriented or even generic programming?

Answer 1

Is casting a base class object to a derived class object given a type flag a good programming design?

Usually no.
It usually indicates a badly defined interface in your base class.

A better way to do it will depend on what you want you want to do with it. If you can define an abstract interface with virtual functions that would be the normal way of solving this problem.

But you seem to be using DisplayEventData as simply a bag of properties so maybe there is no generic interface. In which case I would go with a union (but that choice could change quickly based on other information).

struct DisplayEventData   // struct because it is a property bag not a real class.
{
     DisplayEventType   type;
     union
     {
            WindowEventData   win;
            KeyboardEventData key;
            MouseEventData    mouse;
            JoystickEventData joy;
     } data;
};

PS:

This is the wrong cast:

KeyboardEventData* kev = static_cast<KeyboardEventData>(ev.GetEventData());

You should be using dynamic_cast

Answer 2

Casting a base class object to a derived class object is usually not desired design. Though, it is commonly acceptable as a shortcut approach.

You can consider using Double dispatch and Visitor pattern to handle your event loop. That will be more performance efficient, though is not the same obvious as mentioned shortcut.

class EventDataVisitor
{
public:
    virtual void visit(WindowEventData& event) {};
    virtual void visit(KeyboardEventData& event) {};
    virtual void visit(MouseEventData& event) {};
};

class DisplayEventData
{
public:
    virtual void accept(EventDataVisitor& visitor) = 0;
    virtual ~DisplayEventData();
};

class WindowEventData : public DisplayEventData
{
    virtual void accept(EventDataVisitor& visitor){ visitor.visit(*this); }
};

class KeyboardEventData : public DisplayEventData
{
    virtual void accept(EventDataVisitor& visitor){ visitor.visit(*this); }
};

class MouseEventData : public DisplayEventData
{
    virtual void accept(EventDataVisitor& visitor){ visitor.visit(*this); }
};

class MyKeyboardEventHandler : public EventDataVisitor
{
    void visit(KeyboardEventData& event)
    {
        // Do my stuff
    }
};

void main()
{
    // Poll for next event in the queue
    DisplayEvent ev;
    display->PollEvent(ev); // Fills ev with the needed data

    MyKeyboardEventHandler myHandler;
    ev.GetEventData().accept(myHandler);
}

Answer 3

You may consider looking into the dynamic_cast keyword. which allows polymorphic classes to be down-casted based on their type.

For example:

class Base {
public:
    virtual ~Base() { }
};

class Derived : public Base {

};

int main() {
    Base* base = new Base;
    Base* derived = new Derived;

    cout << dynamic_cast<Derived*>(base) << endl;     // Prints "0" or "nullptr"
    cout << dynamic_cast<Derived*>(derived) << endl;  // Prints the address of the object.
}

This will work because if the base class is polymorphic (has some sort of virtual function) the compiler emits information that makes it possible for dynamic_cast to check what type the object actually is. If it is the correct type, it will perform the cast. If not it will not (returning nullptr).

That said, you should never use switch/cascading if statements with polymorphism. It's better to declare a virtual method that you override in your classes that perform the task instead.

Answer 4

I think I'd be inclined to use a std::unique_pointer as the type of thing returned by the poll and use polymorphism rather than enums. That is,

// Poll for next event in the queue
std::unique_ptr<DisplayEvent> ev{display->PollEvent()};   
ev->doStuff();   // polymorphic call to overloaded doStuff member

This avoids both object slicing and ugly enum/switch construction.

Update

To flesh out this idea a bit more, let's play with animals. I'm hoping that how this is useful will be apparent, but I'm using more concrete names and actions to make it perhaps a bit easier to understand and more general. This code starts with a base class Animal which is analogous to your event base class.

#include <memory>
#include <iostream>
#include <string>

class Animal 
{
    static int serial;
    int id;
public:
    Animal() : id(++serial) {};
    virtual int getId() const { return id; }
    virtual std::string speak() const { return "I cannot speak!"; }
    virtual ~Animal() { std::cout << "goodbye from animal #" << id << '\n'; }
    friend std::ostream &operator <<(std::ostream &out, const Animal& a) {
        return out << a.speak() << '\n';
    }
};

int Animal::serial = 0;

I've added a serial number so that each animal gets a unique id so we can easily tell which instance of a class is doing what.

Next are a few other animal classes, each of which are specializations of the base type.

class Cow : public Animal
{
public:
    Cow() {}
    virtual ~Cow() { std::cout << "~Cow " << getId() << "\n"; }
    std::string speak() const { return "moo!";}
};

class Cat : public Animal
{
public:
    Cat() {}
    virtual ~Cat() { std::cout << "~Cat " << getId() << "\n"; }
    std::string speak() const { return "meow!";}
};

class Dog : public Animal
{
public:
    Dog() {}
    virtual ~Dog() { std::cout << "~Dog " << getId() << "\n"; }
    std::string speak() const { return "woof!";}
};

Now we have a function which returns a pointer to an Animal but, like your event polling loop, we don't know which kind of animal it will be.

Animal* getCritter(int type)
{
    if (0 == type % 3) 
        return new Dog();
    if (0 == type % 5)
        return new Cow();
    return new Cat();
}

Here's the main routine to exercise all of this

int main() 
{
    for (int i=0; i < 12; ++i) {
        std::unique_ptr<Animal> c{getCritter(i)};
        std::cout << "animal #" << c->getId() << " says " << *c;
    }
}

Note that I've used std::unique_ptr<Animal> here. I could just as easily have used a plain pointer here, but std::unique_ptr<> has a few advantages in that the destructor automatically gets called when each Animal goes out of scope (probably the same way you've got no reason to keep already-processed events around) and that I avoid inadvertently making spurious copies of objects because it's actively prevented by std::unique_ptr.

In this case the base class has a friend to create an overload for operator<< in the same way you might have a generic base class member function that processes each event. The base class function calls the overloaded speak() function that gives each animal its own individual unique sound in the same way that different kinds of events will each need to be processed, but each with its own particular routines and data structures.

Answer 5

Is casting a base class object to a derived class object given a type flag a good programming design?

No. Usually it is a symptom of your class hierarchy not being specialized enough (not covering the functionality it should through virtual functions), or your client code being too generic (i.e. your code working on base pointers or refs, when it should be working on a specialized type).