1
\$\begingroup\$

I created a simple C++ event system. In my code Event_channel is a static class that wraps singletons that manages events per event type basis. Every singleton stores a bunch of event handlers which are callbacks fired when the event of its corresponding type is fired. Event_handler_raii is a RAII wrapper of event handlers just like std::lock_guard.

I omit the namespace here. In the real code-base the header are wrapped into a namespace.

Any feedback or suggestions are welcome.

#pragma once

#include <algorithm>
#include <cassert>
#include <vector>
#include <functional>
#include <mutex>
#include <stdexcept>

class Event_channel {
public:
    template <typename Event, typename Handler>
    static void add_handler(Handler& handler);

    template <typename Event, typename Handler>
    static void remove_handler(Handler& handler);

    template<class Event>
    static void broadcast(const Event& evt);
};

template<class Handler>
class Event_handler_raii {
public:
    template<typename... Args>
    Event_handler_raii(Args&&... args);
    ~Event_handler_raii();

private:
    Handler handler_;
};

template<typename Event>
struct Event_handler_trait {
    using event_type = Event;
};


namespace detail {

// A global channel per event type
template<class Event>
class Internal_static_channel {
public:
  static Internal_static_channel& instance();

  template <typename T>
  void add_handler(T& handler);

  template <typename T>
  void remove_handler(T& handler);

  void broadcast(const Event& evt);

private:
  using Handler = std::function<void(const Event&)>;

  std::mutex handlers_mutex_; // Protects handlers
  std::vector<void*> original_ptrs_;
  std::vector<Handler> handlers_;

  Internal_static_channel() {}

  void validate() const {
      assert(original_ptrs_.size() == handlers_.size());
  }
};

template<class Event>
Internal_static_channel<Event>& Internal_static_channel<Event>::instance() {
    static Internal_static_channel result;
    return result; // return a reference to an internal static
}

template<class Event>
template <typename T>
void Internal_static_channel<Event>::add_handler(T& handler) {
    std::lock_guard<std::mutex> lock(handlers_mutex_);
    original_ptrs_.push_back(&handler);
    handlers_.push_back(
      [&handler] (const Event& evt) {
        handler(evt);
      }
    );

    validate();
}

template<class Event>
template <typename T>
void Internal_static_channel<Event>::remove_handler(T& handler) {
    std::lock_guard<std::mutex> lock(handlers_mutex_);
    const auto it = std::find(original_ptrs_.begin(), original_ptrs_.end(),
                              &handler);
    if (it == original_ptrs_.end()) {
        throw std::runtime_error {
            "Tried to remove a handler that was not in the handler list"
        };
    }

    const auto index = it - original_ptrs_.begin();
    std::iter_swap(it, original_ptrs_.end() - 1);
    std::iter_swap(handlers_.begin() + index, handlers_.end() - 1);
    original_ptrs_.pop_back();
    handlers_.pop_back();

    validate();
}

template<class Event>
void Internal_static_channel<Event>::broadcast(const Event& evt) {
    std::vector<Handler> local_queue(handlers_.size());
    {
        std::lock_guard<std::mutex> lock(handlers_mutex_);
        local_queue = handlers_;
    }

    for (auto& handler: local_queue)
        handler(evt);
}

} // namespace detail

template<typename Event, typename Handler>
void Event_channel::add_handler(Handler& handler)
{
    detail::Internal_static_channel<Event>::instance().add_handler(handler);
}

template<typename Event, typename Handler>
void Event_channel::remove_handler(Handler& handler) {
    detail::Internal_static_channel<Event>::instance().remove_handler(handler);
}

template<class Event>
void Event_channel::broadcast(const Event& evt) {
    detail::Internal_static_channel<Event>::instance().broadcast(evt);
}

template<class Handler>
template<typename... Args>
Event_handler_raii<Handler>::Event_handler_raii(Args&&... args)
    : handler_{Handler{std::forward<Args>(args)...}} {
    Event_channel::add_handler<Handler::event_type>(handler_);
}

template<class Handler>
Event_handler_raii<Handler>::~Event_handler_raii() {
    Event_channel::remove_handler<Handler::event_type>(handler_);
}

Below is a simple use case. Note that all event handler types need to inherent from Event_handler_trait template, or we lose the ability to use Event_handler_raii template. This fact means an event handler must be a function object instead of other form of callable (like lambda).

struct Test_event {
  int value;
};

class Test_event_handler : public Event_handler_trait<Test_event> {
public:
    Test_event_handler(std::stringstream& sstream) : ss_{sstream} {}

    void operator()(const event_type& evt) {
        ss_ << "Event received: " << evt.value;
    }

private:
    std::stringstream& ss_;
};

int main() {
    std::stringstream ss;
    Event_handler_raii<Test_event_handler> handler(ss);
    Event_channel::broadcast(Test_event{456}); // ss now contains "456"
}

Edit: I want to get rid of the singletons and just pass Event_channel around. But than how do I store event handler of different types under a generic interface?

\$\endgroup\$
  • \$\begingroup\$ But, why are you writing your own event handler system? Or rather, what for? Who's going to be "broadcasting" events? etc. \$\endgroup\$ – einpoklum Aug 22 '17 at 20:49
2
\$\begingroup\$

I am not a big user of metaprogramming, mostly i have found on the layers that I work on i don't need the amount of flexibility that is being offered. But i see a couple of issues not so much with the design but the functionality. I'll leave the review of the template architecture to others.

  • Looks like you could be broadcasting messages to handlers that are currently under destruction, the handler could get removed via the call in the descructor of the raii wrapper. But it would still exist in the copy of the handler list used for broadcasting. This means that it could receive an event while the destructor is being processed.

  • When running through the static channels it looks like that events could be received out of order if the broadcast comes from different threads. This would be something that would need to be documented.

  • Broadcast blocks, this means that there is an implicit requirement for event handlers to keep their processing "short" with respect to the system needs. This could be alleviated by executing broadcast asynchronously but then the above bullet becomes even more of an issue

  • It seems that using traits seems to complicate the handling, rather than being able to use Handler<Event> inside the Internal_static_channel you are forced keep the pointer separately and create an additional lambda to wrap the call. But see note with regard to template programming above ...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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