Writing a very simple event class in C++

Question

I'm trying to write a really simple event or message class in C++. I want the event to hold the time of occurrence and some data which are specific to the event type. What I have at the moment is the following

class EventData{
public:
    virtual ~EventData(void) = 0;
};

struct Event{
    Event(EventType type, Time time, EventData *data = nullptr):
        type_(type), time_(time), data_(data)
    {}
    ~Event(void){
        if(data_) delete data_;
    }
    //Disable copying, event memory will be managed by EventManager
    Event(const Event& other) = delete;

    const EventType  type_;
    const Time       time_;
    const EventData *data_;
};

In my main loop, I have something like this,

bool running = true;
while(running){
    const Event* nextEvent = evtManager.getNextEvent();
    switch(nextEvent.type_){
    case EVT_A:
        const EventAData* data = static_cast&ltEventAData*&gt(nextEvent.data_);
        //do stuff
        break;
    }
    ...
    case EVT_END:
        running = false;
        break;
    }
}

The question then is if there is a more efficient way of doing this, i.e. with templates. the other problem is that someone could accidentally give the wrong EventType, EventData pair, in which case the static_cast will fail.

I should note that I want this Event class to be as fast as possible, especially the access to the time_ member variable.

Answer 1

1. There is no need to check whether the pointer is null before deleting it.
2. You are trying to perform type-erasing, and then perform different tasks depending on the type you just erased. It's inefficient. Look into boost::variant instead, it seems that it's exactly what you need.
3. If you insist on using this method, you should use typeid to discriminate between types.

Answer 2

Instead of using templates, you could use inheritance.

Event can be an abstract base class with one member variable, _time. Accessing the _time member variable will be fast with a simple accessor method (possibly inlined) or by making _time a public member variable. You’re almost always better off using an accessor method. The access will be plenty fast, and the accessor will allow you the flexibility to change your internal representation of the time at some later point if the need arises.

The EventType enum in your example really describes all the possible subclasses of Event. If you were to create a concrete subclass of the abstract base class Event for every entry in EventType, you really wouldn’t need the EventType enum. The subclass identifies the event type. The constructors for all the subclasses could be unique, each one taking different parameters. Each subclass could provide accessors for its member variables. These additional constructor parameters and accessors could replace the EventData class.

Finally, you have two alternatives for your main loop. First, you might treat the processing currently done in each case statement polymorphically. The Event base class could have a pure virtual function called “processEvent()” which each subclass could override. If that is possible, you could replace the switch statement with a simple function call:

nextEvent->processEvent();

Second, if that isn’t feasible, you could use RTTI to downcast the nextEvent variable from an Event pointer to a pointer of the appropriate subclass. You could then call the subclass-specific accessors to perform processing that’s similar to what you’re currently doing.