Simple event system

Question

I made this simple event system:

template<typename Func, typename Args>
class Event
{
public:
    static void dispatch(Args args) noexcept
    {
        for (auto listener : listeners)
            listener.second(args);
    }

    static void attach(const std::string& id, Func func) noexcept
    {
        listeners.insert({ id, func });
    }

    static void dettach(const std::string& id) noexcept
    {
        listeners.erase(id);
    }

private:
    static std::unordered_map<std::string, Func> listeners;
};

template<typename Func, typename Args>
std::unordered_map<std::string, Func> Event<Func, Args>::listeners;

What I wanted to do is to let me create any sort of event without needing to do a lot more code.

so for example in this code, if I want to add a KeyEvent, for example, I could do this:

struct KeyArguments {
    int key;
    int state;
};

class KeyEvent : public Event<std::function<void(const KeyArguments&)>, KeyArguments>
{
};

And to use such code:

KeyArguments args;
args.key = Key::A;
args.state = State::KeyUp;

KeyEvent::attach("new", func);
KeyEvent::attach("new2", [](KeyArguments args) { std::cout << "The key that was pressed is " << args.key;});    
KeyEvent::dispatch(args);
KeyEvent::dettach("new2");
KeyEvent::dispatch(args);

I also wanted to make the design reachable from anywhere in the code, this is why I used static keyword, and this is the part that doesn't really "feel" good.

Should I really be using static in this case? Is there any cons in using such design?

I feel like using templatefor this case is a bit overkill. For each event create a "new" class (the struct doesn't bother me). Do you think there is a way around that? Is it as bad as I think it is?

EDIT: The design goal was to create an event system that anyone could attach to and get notified when the specific event has happened. For example in a game we would like to distribute an event for KeyPress, so the event system (in the engine) would let the game programmer attach the Player::input function to it, and if he would like, Menu::input as well. Both of them will get notified when a KeyPress event has happened.

Answer 1

There is one big problem with your class: You can't have two different events with the same parameters in the functions. This is a real problem, as registering functions with no arguments is common in events. It would be better to actually use the id (the std::string you do not use), to trigger the event you actually want.

Manually creating a class every time you need a new type of function pointer isn't really a solution, especially when templates can do this for you.

I'm not sure why you don't directly use std::function in the Event class, but can't you have another generic class that you can use every time you want an Event with an std::function? Something like this:

template<typename Args>
class EventStdFunction : public Event<std::function<void(Args)>, Args>

I didn't try that but I don't see why it wouldn't work.

You said in comment that you used an unerdored_map so that you can put any arguments in your function (through a struct). Using variadic templates will let you have more flexible parameters lists in your functions. You won't have to necessarily use container class like your KeyArgument. Something like this:

template<typename... Args>
class EventStdFunction : public Event<std::function<void(Args...)>, Args...>


template<typename Func, typename... Args>
class Event
{
public:
    static void dispatch(Args... args) noexcept
    {
        for (auto listener : listeners)
            listener.second(args);
    }

    static void attach(const std::string& id, Func func) noexcept
    {
        listeners.insert({ id, func });
    }

    static void dettach(const std::string& id) noexcept
    {
        listeners.erase(id);
    }

private:
    static std::unordered_map<std::string, Func> listeners;
};

If you use variadic templates, you can define a delegate class used in your Event class and drop the EventStdFunction option. So instead of having to specify a type as a template parameter in your Event class, you'd use a delegate class that store all the function pointers you need for the specific event. So you'd have something like this:

template<typename... Args>
class Event
{
public:
    static void dispatch(std::string& id, Args... args) noexcept
    {
        listeners[id].call(args);
    }

    static void attach(const std::string& id, Delegate<Args...> func) noexcept
    {
        listeners.insert({ id, func });
    }

    static void dettach(const std::string& id) noexcept
    {
        listeners.erase(id);
    }

private:
    static std::unordered_map<std::string, Delegate<Args...> > listeners;
};

Inside your Delegate class you could use an std::vector<std::function<void(Args...)> > for example to store all your function pointers.

I'm not sure about the static part. I'd probably use a singleton pattern here if I were you, but the only benefit I see in this is that you control when the singleton is instantiated. If you don't need this, it doesn't really matter.

Edit: The for loop was useless in the last example in the dispatch function. Note that you should check if the map contains a valid delegate before calling it. In this example, the Delegate would need a call function.