Simple event dispatcher

Question

Here is my very first event dispatcher. I would like to get both, style and code review, as well as some ideas to improve this implementation (new features etc.)

I tried to write code in C++17 style as much as possible.

IEventDispatcher.h

#pragma once

#include <map>
#include <functional>

template<typename EventType>
class IEventDispatcher
{
public:
    using EventHandler = std::function<void()>;

    virtual void addEventListener(EventType eventToAdd, EventHandler handler) = 0;
    virtual void removeEventListener(EventType eventToRemove) = 0;
    virtual void dispatch(EventType eventToDispatch) = 0;
};

EventDispatcher.hpp

#pragma once

#include "IEventDispatcher.h"

template<typename EventType>
class EventDispatcher : public IEventDispatcher<EventType>
{
public:
    using EventHandler = std::function<void()>;

    EventDispatcher();
    ~EventDispatcher();

    void addEventListener(EventType eventToAdd, EventHandler handler);
    void removeEventListener(EventType eventToRemove);
    void dispatch(EventType eventToDispatch);

private:
    std::multimap<EventType, EventHandler> eventListeners;
};

template<typename EventType>
EventDispatcher<EventType>::EventDispatcher()
{}

template<typename EventType>
EventDispatcher<EventType>::~EventDispatcher()
{}

template<typename EventType>
void EventDispatcher<EventType>::addEventListener(EventType eventToAdd, EventHandler handler)
{
    eventListeners.emplace(eventToAdd, handler);
}

template<typename EventType>
void EventDispatcher<EventType>::removeEventListener(EventType eventToRemove)
{
    eventListeners.erase(eventToRemove);
}

template<typename EventType>
void EventDispatcher<EventType>::dispatch(EventType eventToDispatch)
{
    for (const auto &[event, handler] : eventListeners)
    {
        if (event == eventToDispatch)
        {
            handler();
        }
    }
}

Answer 1

Design

• Is there a need for IEventDispatcher? Defining it as an abstract base class doesn't provide much utility if there's only ever going to be one derived class.

• Is there any requirement for EventDispatcher::eventListeners to be of type std::multimap<EventType, EventHandler>? It doesn't seem like the ordering of elements has much relevance, so a std::unordered_multimap<EventType, EventHandler> might provide better performance. But: The current code doesn't even use any std::multimap specific functionality, so the easiest solution might actually be a std::unordered_map<EventType, std::vector<EventHandler>>.

• There is no way to remove a specific EventHandler, only all EventHandlers registered for an EventType.

• There's no way to pass arguments to an event handler. This might be necessary for some use cases, e.g. to indicate what changed.

  There's no easy way to provide both general event dispatching (handling multiple different EventTypes) and event-specific parameter passing (different parameter types for different events). Some solutions:

  • Redefining the scope of EventDispatcher: Instead of handling multiple events in each EventDispatcher instance each instance is only responsible for handling one specific EventType. The parameter types can then be taken as template parameters.

  • Make EventType a base class and inherit from it when parameters need to be passed (so more like EventArgs in C#). Requires EventType instances to be passed as references or pointers, and EventHandlers need to downcast internally to the concrete derived class. Many possible ways to get this wrong, and the compiler might not be able to help much.

Implementation issues

• EventDispatcher::dispatch iterates over all entries in EventDispatcher::eventListeners. Why not use std::multimap::equal_range to then only iterate over all entries that match?

  This would be \$\mathcal{O}(k + \log n)\$ on average instead of \$\mathcal{O}(n)\$ (where \$n\$ denotes the number of all elements in the container, and \$k\$ denotes all elements for the relevant key), or \$\mathcal{O}(k)\$ on average for std::unordered_multimap.

  The proposed replacement std::unordered_map<EventType, std::vector<EventHandler>> would also have complexity \$\mathcal{O}(k)\$ (\$\mathcal{O}(1)\$ to find the vector, \$\mathcal{O}(k)\$ to iterate it) - even in the worst case.

• Rule of Five violation: A custom destructor is declared for EventDispatcher, but no custom copy constructor, copy assignment operator, move constructor and move assignment operators are declared. This prevents the compiler from providing default implementations for those (or soon will, it currently is deprecated for compilers to do so in case of copy constructor and copy assignment operator).

  It would be easiest to just use the default compiler-provided destructor.

• Inconsistent naming: IEventDispatcher.h has file extension .h, whereas EventDispatcher.hpp has file extension .hpp.

• In EventDispatcher, all member functions inherited and overridden from IEventDispatcher lack the override keyword.

• EventDispatcher::dispatch could be const.