7
\$\begingroup\$

I am learning C++ and have been trying to create an event system for use in a small game. This will be the mechanism by which game entities communicate. I would be eternally grateful if someone with experience could critique my code. Running through it myself, I'm a little concerned about the use of Runtime Type Information (RTTI). I know that this is discouraged but I can't find any alternative to providing some basic type safety.

class EventDispatcher
{
public:
    /**
     * Callback function type for event handling.
     */
    template<typename EventType>
    struct Callback
    {
        /**
         * Constructor.
         */
        Callback(boost::function<void(const EventType& event)> callback) :
        handler(callback)
        {
            // Nothing to do.
        }

        /**
         * Callback function.
         */
        boost::function<void(const EventType& event)> handler;
    };

    /**
     * Constructor.
     */
    EventDispatcher();

    /**
     * Destructor.
     */
    ~EventDispatcher();

    /**
     * Subscribe to receive events.
     *
     * @param callback Event handler callback.
     */
    template <typename EventType>
    void Subscribe(Callback<EventType> callback)
    {
        GetSpecificDispatcher<EventType>()->Subscribe(callback);
    }

    /**
     * Immediately broadcast an event to subscribers.
     *
     * @param args... Event constructor arguments.
     */
    template <typename EventType, typename... Arguments>
    void Broadcast(const Arguments... args)
    {
        GetSpecificDispatcher<EventType>()->Broadcast(EventType(args...));
    }

    /**
     * Queue an event to be broadcast to subscribers on the next Update
     * call.
     *
     * @param args... Event constructor arguments.
     */
    template <typename EventType, typename... Arguments>
    void Enqueue(const Arguments... args)
    {
        GetSpecificDispatcher<EventType>()->Enqueue(EventType(args...));
    }

    /**
     * Broadcasts all events in the queue to subscribers.
     */
    void Update()
    {
        for (auto iter = dispatchers.begin(); iter != dispatchers.end(); ++iter)
        {
            iter->second->Update();
        }
    }

protected:
    /**
     * Event specific dispatcher interface.
     */
    class ISpecificDispatcher
    {
    public:
        virtual void Update() = 0;
    };

    /**
     * Event specific dispatcher.
     */
    template<typename EventType>
    class SpecificDispatcher : public ISpecificDispatcher
    {
    public:
        /**
         * Subscribe to receive events.
         *
         * @param callback Event handler callback.
         */
        void Subscribe(Callback<EventType> callback)
        {
            callbacks.push_back(callback);
        }

        /**
         * Immediately broadcast an event to subscribers.
         *
         * @param args... Event constructor arguments.
         */
        template <typename... Arguments>
        void Broadcast(const Arguments... args)
        {
            EventType event(args...);
            for (const Callback<EventType>& callback : callbacks)
            {
                callback.handler(event);
            }
        }

        /**
         * Queue an event to be broadcast to subscribers on the next Update
         * call.
         *
         * @param args... Event constructor arguments.
         */
        template <typename... Arguments>
        void Enqueue(const Arguments... args)
        {
            eventQueue.push(EventType(args...));
        }

        /**
         * Broadcasts all events in the queue to subscribers.
         */
        virtual void Update()
        {
            while (!eventQueue.empty())
            {
                const EventType& event = eventQueue.front();
                for (const Callback<EventType>& callback : callbacks)
                {
                    callback.handler(event);
                }
                eventQueue.pop();
            }
        }

    private:
        /**
         * Queued events.
         */
        std::queue<EventType> eventQueue;

        /**
         * Event callback handlers.
         */
        std::vector<Callback<EventType>> callbacks;
    };

    /**
     * Returns a pointer to the specific dispatcher for the event type.
     * A specific dispatcher is first created if one does not exist.
     */
    template<typename EventType>
    boost::shared_ptr<SpecificDispatcher<EventType>> GetSpecificDispatcher()
    {
        auto iter = dispatchers.find(typeid(EventType).name());
        if (iter == dispatchers.end())
        {
            // A specific dispatcher was not found.
            // We better create one!
            boost::shared_ptr<SpecificDispatcher<EventType>> specificDispatcher(new SpecificDispatcher<EventType>());
            dispatchers[typeid(EventType).name()] = specificDispatcher;
            return specificDispatcher;
        }
        else
        {
            return boost::static_pointer_cast<SpecificDispatcher<EventType>>(iter->second);
        }
    }

private:
    /**
     * Event specific dispatchers.
     */
    std::map<std::string, boost::shared_ptr<ISpecificDispatcher>> dispatchers;
};
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

I'm going to update to C++20 as we go along, beginning with using modern Standard Library rather than Boost.

We need to include some headers:

#include <functional>
#include <map>
#include <memory>
#include <queue>

EventDispatcher declares a constructor and destructor but never defines them (along with comments that add absolutely zero value):

/**
 * Constructor.
 */
EventDispatcher();

/**
 * Destructor.
 */
~EventDispatcher();

We could define them inline with =default, but I think it's better to omit them entirely and let the compiler generate implicit ones.

Conversely, ISpecificDispatcher doesn't have a destructor, but really needs one, to make it a virtual destructor, since it's used as a base class:

class ISpecificDispatcher
{
public:
    virtual ~ISpecificDispatcher() = default;
    virtual void Update() = 0;
};

Without this, delete of a base-class pointer will fail to call subclass destructors.


Now we've dealt with compiler warnings, let's look at the interface. I'd like to be able to subscribe my function:

d.Subscribe([](const int& i){ std::cout << i << '\n'; });

But the function only accepts EventDispatcher::Callback objects:

d.Subscribe(EventDispatcher::Callback<int>{[](const int& i){ std::cout << i << '\n'; }});

We can make that a bit easier to use, with an overload of Subscribe:

template<typename EventType, typename F>
void Subscribe(F&& callback)
{
    Subscribe(Callback<EventType>{std::forward<F>(callback)});
}

which gets us a bit closer:

d.Subscribe<int>([](const int& i){ std::cout << i << '\n'; });

The Callback class seems too heavyweight. I simply replaced with

template<typename EventType>
using Callback = std::function<void(const EventType& event)>;

I don't like the construction of the event within Broadcast() and Enqueue. This forces the use of (…) rather than {…}, and doesn't seem to save anything at the call site. If we simply accept a ready-constructed EventType, like this:

template<typename EventType>
void Enqueue(EventType&& event)
{
    GetSpecificDispatcher<EventType>()->Enqueue(std::forward<EventType>(event));
}

then the call site changes from

d.Enqueue<event>(1,2);

to

d.Enqueue(event{1,2});

That's no longer, and gives the caller more options for constructing the event (which could be especially useful for factory-constructed event objects, which might not have public constructors).

There's certainly no need for SpecificDispatcher to work with anything other than EventType, since that's what its methods are always called with:

    /**
     * Immediately broadcast an event to subscribers.
     */
    void Broadcast(const EventType& event)
    {
        for (auto const& callback: callbacks) {
            callback.handler(event);
        }
    }

    /**
     * Queue an event to be broadcast to subscribers on the next Update
     * call.
     */
    void Enqueue(EventType event)
    {
        eventQueue.push(std::move(event));
    }

Update() repeats the logic of Broadcast() in its inner loop - better to simply call it:

    /**
     * Broadcast all events in the queue to subscribers.
     */
    virtual void Update()
    {
        while (!eventQueue.empty()) {
            auto const& event = eventQueue.front();
            Broadcast(event);
            eventQueue.pop();
        }
    }

GetSpecificDispatcher can be simpler, if we take advantage of operator[] creating a default (i.e. null pointer) object as value:

template<typename EventType>
std::shared_ptr<SpecificDispatcher<EventType>> GetSpecificDispatcher()
{
    auto& specificDispatcher = dispatchers[typeid(EventType).name()];
    if (!specificDispatcher) {
        specificDispatcher.reset(new SpecificDispatcher<EventType>());
    }
    return std::static_pointer_cast<SpecificDispatcher<EventType>>(specificDispatcher);
}

It's wasteful to do string comparisons here - just use typeid directly as the key type (okay, we need to wrap a reference for this):

#include <typeinfo>

namespace typeinfo
{
    using ref = std::reference_wrapper<const std::type_info>;

    struct hash {
        auto operator()(ref code) const {return code.get().hash_code();}
    };
    struct equality {
        auto operator()(ref lhs, ref rhs) const {return lhs.get() == rhs.get();}
    };
}
private:
    using DispatcherPointer = std::shared_ptr<ISpecificDispatcher>;

    std::unordered_map<typeinfo::ref, DispatcherPointer,
                       typeinfo::hash, typeinfo::equality
                       > dispatchers = {};

There's no means to unsubscribe a callback when it's no longer needed. I think that subscribe() perhaps ought to return a token that can be used for subsequent removal.


Modified code

Applying these changes, we arrive at something that may be a better start for moving to compile-time event types:

#include <functional>
#include <map>
#include <memory>
#include <queue>
#include <typeinfo>

namespace typeinfo
{
    using ref = std::reference_wrapper<const std::type_info>;

    struct hash {
        auto operator()(ref code) const
        { return code.get().hash_code(); }
    };
    struct equality {
        auto operator()(ref lhs, ref rhs) const
        { return lhs.get() == rhs.get(); }
    };
}

class EventDispatcher
{
    /**
     * Callback function type for event handling.
     */
    template<typename EventType>
    using Callback = std::function<void(const EventType& event)>;

public:
    template<typename EventType, typename F>
    void subscribe(F&& callback)
    {
        GetDispatcher<EventType>()->subscribe(Callback<EventType>{std::forward<F>(callback)});
    }

    /**
     * Immediately broadcast an event to subscribers.
     *
     * @param args... Event constructor arguments.
     */
    template <typename EventType>
    void broadcast(const EventType& event)
    {
        GetDispatcher<EventType>()->broadcast(event);
    }

    /**
     * Queue an event to be broadcast to subscribers on the next Update
     * call.
     */
    template<typename EventType>
    void enqueue(EventType&& event)
    {
        GetDispatcher<EventType>()->enqueue(std::forward<EventType>(event));
    }

    /**
     * Broadcasts all events in the queue to subscribers.
     */
    void update()
    {
        for (auto& d: dispatchers) {
            d.second->update();
        }
    }

protected:
    /**
     * Event specific dispatcher interface.
     */
    class DispatcherBase
    {
    public:
        virtual ~DispatcherBase() = default;
        virtual void update() = 0;
    };

    /**
     * Event specific dispatcher.
     */
    template<typename EventType>
    class Dispatcher : public DispatcherBase
    {
        std::vector<Callback<EventType>> callbacks = {};
        std::queue<EventType> eventQueue = {};

    public:
        void subscribe(Callback<EventType> callback)
        {
            callbacks.push_back(std::move(callback));
        }

        void broadcast(const EventType& event)
        {
            for (auto const& callback: callbacks) {
                callback(event);
            }
        }

        void enqueue(EventType event)
        {
            eventQueue.push(std::move(event));
        }

        void update() override
        {
            while (!eventQueue.empty()) {
                auto const& event = eventQueue.front();
                broadcast(event);
                eventQueue.pop();
            }
        }
    };

    /**
     * Returns a pointer to the specific dispatcher for the event type.
     * A specific dispatcher is first created if one does not exist.
     */
    template<typename EventType>
    std::shared_ptr<Dispatcher<EventType>> GetDispatcher()
    {
        auto& specificDispatcher = dispatchers[typeid(EventType)];
        if (!specificDispatcher) {
            specificDispatcher.reset(new Dispatcher<EventType>());
        }
        return std::static_pointer_cast<Dispatcher<EventType>>(specificDispatcher);
    }

private:
    using DispatcherPointer = std::shared_ptr<DispatcherBase>;

    std::unordered_map<typeinfo::ref, DispatcherPointer,
                       typeinfo::hash, typeinfo::equality
                       > dispatchers = {};
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.