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.

Wow. This is really complex. You are really reinventing the wheel. Why don't you use a very simple Observer pattern. I don't see how this has any other benefits over that. The purpose of a class is to define attributes and methods instead you have passed in your "method" to the KeyEvent class. — NastyDiaper
– NastyDiaper, Commented Dec 21, 2016 at 20:59
@jnbbender I didn't said it is complex... that's why I said "SIMPLE event system"... I didn't like the Observer pattern so I didn't go with that... — Zik332
– Zik332, Commented Dec 22, 2016 at 11:06
Your code doesn't seem to work. 1) Your example doesn't use the same signature for attach than the one defined in your Event class. 2) You didn't explain what you want, but usually an event manager uses the id to raise a specific event i.e. call all the function pointers registered for a specific event id. Here, your dispatch function doesn't use the std::string key in the map. So when you dispatch events, the class will dispatch all the events with the same parameters (all the function pointers that this class stored), whatever the name of the event is. — Stud
– Stud, Commented Dec 22, 2016 at 11:42
1) Thanks for mentioning that, the moderator edited my question so it got wrong, I'll fix that now. 2) The name (and map) is meant for detaching a specific function, I needed a class that would let me put any arguments I want so I went with the struct of arguments. Thanks for the comment! — Zik332
– Zik332, Commented Dec 22, 2016 at 11:46
@Stud And I called every function that attached to that specific event because that basically what I wanted to happen, for example, KeyEvent (in a game) will call every function that attached to it to let them know a key was pressed. — Zik332
– Zik332, Commented Dec 22, 2016 at 11:52

Stud · Accepted Answer · 2016-12-22 13:10:56Z

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.

Thanks! Didn't think about the problem you mentioned! I'll try to use the id like you said and give it a shot with template variadic. So basically I could do this in your solution: class KeyEvent : public EventStdFunction<int, int> {}; — Zik332
– Zik332, Commented Dec 22, 2016 at 12:51
I cant seem to have a vector with std::function<Args...> because it is variadic I guess. I'll fix this though, Thanks again for the review! — Zik332
– Zik332, Commented Dec 22, 2016 at 12:52
This is valid: std::vector<std::function<void(Arguments...)> > funcs; — Stud
– Stud, Commented Dec 22, 2016 at 12:53
Added listener.second(std::forward<Args>(args)...); and working like a charm :) — Zik332
– Zik332, Commented Dec 22, 2016 at 12:58

Stack Exchange Network

Simple event system

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Simple event system

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions