C++ 14 event bus implementation

Question

The code allows you to subscribe to events and publish events via an EventBus class. Events are identified by a source and subject. Events can have a single parameter or they can be 'indexed' and have two parameters (the first being an integer index - I will be using this a lot so wanted a specialization).

I don't particularly like the fact that:

1. I need to have an IndexedSubscriptionmethod - it would be nicer to overload the Subscription method, but the compiler doesn't like that.
2. The template argument needs to be specified when calling Subscription and IndexedSubscription.

I would like to know if there are any improvements I could make (I am reasonably new to C++ but am experienced in other languages).

Example use

#include <iostream>
#include <typeinfo>
#include "EventBus.h"

class Sources
{
public:
    static EventSource Source1()
    {
        static EventSource source;

        return source;
    }

    static EventSource Source2()
    {
        static EventSource source;

        return source;
    }
};

class Subjects
{
public:
    static EventSubject<double> Subject1()
    {
        static EventSubject<double> subject;

        return subject;
    }
};

int main()
{
    EventBus eb;

    eb.Subscribe<double&>(Sources::Source1(), Subjects::Subject1(), [](double& data)
    {
        std::cout << "data: " << data << std::endl;
    });

    eb.Publish<double>(Sources::Source1(), Subjects::Subject1(), 33);

    std::cout << "Done... ";
    std::getchar();

    return EXIT_SUCCESS;
}

Main event bus source

#pragma once

#include <atomic>
#include <vector>
#include <map>
#include <memory>
#include <functional>

template<class TData>
using EventHandler = std::function<void(TData)>;

template<class TData>
using IndexedEventHandler = std::function<void(int, TData)>;

template<class T>
using EventDataType = typename std::remove_reference<typename std::remove_reference<T>::type>::type;

class EventIdentifier
{
private:
    int _id;

public:
    EventIdentifier()
    {
        static std::atomic_int counter = 1;

        _id = counter++;
    }

    bool operator ==(const EventIdentifier &other) const
    {
        return _id == other._id;
    }

    bool operator<(const EventIdentifier& other) const
    {
        return _id < other._id;
    }
};

class EventSource final : public EventIdentifier
{
};

template<class TData>
class EventSubject final : public EventIdentifier
{
};

template<class TData>
class IndexedData
{
private:
    int _index;
    EventDataType<TData>& _data;

public:
    IndexedData(int index, EventDataType<TData>&& data) :
        _index(index), _data(data)
    {
    }

    int GetIndex()
    {
        return _index;
    }

    EventDataType<TData>& GetData()
    {
        return _data;
    }
};

class EventBus
{
private:
    class EventObserver
    {
    public:
        virtual void Notify(void* data) = 0;
    };

    template<class TData>
    class EventSubscription : public EventObserver
    {
    private:
        EventHandler<TData> _observer;

    public:
        EventSubscription(EventHandler<TData> observer)
        {
            _observer = observer;
        }

        virtual void Notify(void* data) override
        {
            EventDataType<TData>* pointer = static_cast<EventDataType<TData>*>(data);

            _observer(*pointer);
        }
    };

    class EventKey
    {
    private:
        EventIdentifier _source;
        EventIdentifier _subject;

    public:
        EventKey(EventIdentifier source, EventIdentifier subject) :
            _source(source), _subject(subject)
        {

        }

        bool operator<(const EventKey& other) const
        {
            if (_source < other._source)
            {
                return _subject < other._subject;
            }
            else
            {
                return false;
            }
        }
    };

    std::map<EventKey, std::vector<std::unique_ptr<EventObserver>>> _observers;

public:
    template<class TData>
    void Subscribe(EventSource source, EventSubject<EventDataType<TData>> subject, EventHandler<TData> observer)
    {
        Subscribe(EventKey(source, subject), observer);
    }

    template<class TData>
    void IndexedSubscribe(EventSource source, EventSubject<EventDataType<TData>> subject, IndexedEventHandler<TData> observer)
    {
        Subscribe<IndexedData<TData>&>(EventKey(source, subject), [observer](IndexedData<TData>& data)
        {
            observer(data.GetIndex(), data.GetData());
        });
    }

    template<class TData>
    void Publish(EventSource source, EventSubject<EventDataType<TData>> subject, TData&& data)
    {
        Publish(EventKey(source, subject), std::forward<TData>(data));
    }

    template<class TData>
    void IndexedPublish(EventSource source, EventSubject<EventDataType<TData>> subject, int index, EventDataType<TData>&& data)
    {
        Publish(EventKey(source, subject), IndexedData<TData>(index, std::forward<EventDataType<TData>>(data)));
    }

private:
    template<class TData>
    void Subscribe(EventKey key, EventHandler<TData> observer)
    {
        auto& subscripions = _observers[key];

        subscripions.push_back(std::make_unique<EventSubscription<TData>>(observer));
    }

    template<class TData>
    void Publish(EventKey key, TData&& data)
    {
        auto found = _observers.find(key);

        if (found != _observers.end())
        {
            for (auto& observer : found->second)
            {
                observer->Notify(&data);
            }
        }
    }
};

Answer 1

Missing includes

The header needs

#include <type_traits>
#include <utility>

And the demo program needs

#include <cstdio>

(or - better - just omit the final std::getchar() whose result is never used and whose error is ignored)

---

It's not clear why EventDataType removes references twice - C++ doesn't have references to references.

Instead of transforming types like that, it's probably better to constrain the template functions to reject reference types as arguments.

---

IndexedData::_data looks very dangerous, as it's a reference that's initialised from an rvalue-reference argument, so could very easily be invalidated.

---

I don't like laundering event data through void*, but you seem to have protected against accidents by carrying the type with the Subject class.

I think we could avoid casting, by using separate maps for subscriptions of different types.

---

EventBus::EventObserver has a virtual function, so we should provide it with a public virtual destructor for safety. Also, I don't think it makes sense for the Notify to modify the observer itself:

    struct EventObserver
    {
        virtual ~EventObserver() = default;
        virtual void Notify(void* data) const = 0;
    };

---

Consider making EventBus::EventSubscription final, which could improve the compiler's ability to optimise.

---

I see no way to unsubscribe from this publisher. That's problematic, particularly if we have subscriptions which have captured references.

---

Instead of the separate Indexed versions of functions, consider generalising to take any number of data arguments, and forwarding them as a parameter pack. That reduces code and provides more user flexibility!

---

The test program looks very long-winded when creating source and subject identifiers. It can be simplified:

namespace Sources
{
    const EventSource Source1;
    const EventSource Source2;
}

namespace Subjects
{
    const EventSubject<double> Subject1;
}

We needn't worry about the exact order in which these globals are initialised; any order is fine.

---

Consider adding names to our sources and subjects, because debugging gets hard when events are only identified by integers (and not easily predictable ones at that!).

---

The test program shouldn't exit with a partially-written line of output - add \n to the "Done" string.