Better system than having multiple vectors for each event type

Viewed 63

I am trying to implement a basic event system. This event system consists of a series of void pointers used for callback functions. The code below is BAD design. If I want to add many events, the code quickly gets bloated.

Currently, the bad solution needs to know what vector to add to, and which vector to run based off of the event type. Ideally, I could store all the void pointers in a single vector, and based off of the type of event that occurs, the function pointers using that event could be run.

If anyone has any ideas they would like to give for a better implementation, it would be greatly appreciated.

Example event handler class:

#include <vector>

typedef void(*CALLBACK)();

enum EventType {

    WINDOW_RESIZE,
    WINDOW_CLOSE,
};

class EventManager {

    public:

        // Add a callback function
        void bind(CALLBACK f, EventType type) {

            switch(type) {

                // Note I would need to implement a case for all the events
                case WINDOW_RESIZE: {
                    window_resize_event.push_back(f);
                }
                ///...
            }
        }

        // This would poll the event queue, and based on the
        // event, run the callback functions
        void update() const {

            unsigned int event;

            if (event == WINDOW_RESIZE) {
                ///iterate through vector
            }
            ///...
        }

    private:

        std::vector <callback> window_resize_event;
        std::vector <callback> window_close_event;
};

Edit: The implemented solution using a multi map

#include <stdio.h>

#include "event.hpp"

EventManager::EventManager() {

    std::vector <CALLBACK> callbacks;

    // Copy callbacks vector into each
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (WINDOW_CLOSE  , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (WINDOW_RESIZE , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (KEY_DOWN      , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (KEY_UP        , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (MOUSE_DOWN    , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (MOUSE_UP      , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (MOUSE_MOVED   , callbacks));
    events.insert(std::pair <EventType, std::vector <CALLBACK>> (MOUSE_SCROLLED, callbacks));
}

void EventManager::bind(const CALLBACK &f, const EventType &type) {

    std::multimap <EventType, std::vector <CALLBACK>>::iterator it;

    for (it = events.begin(); it != events.end(); it++) {

        if (it->first == type) {
            it->second.push_back(f);
            return;
        }
    }

    printf("Unknown event type\n");
}

void EventManager::update() {

    std::multimap <EventType, std::vector <CALLBACK>>::iterator it;

    while (SDL_PollEvent(&event)) {

        for (it = events.begin(); it != events.end(); it++) {

            // Cast event to the SDL equivalent
            if (event.type == static_cast <uint32_t> (it->first)) {

                run(it->second);
            }
        }
    }
}

void EventManager::run(const std::vector <CALLBACK> &callbacks) {

    // Call the registered functions
    for (unsigned int i = 0; i < callbacks.size(); i++) {
        (*callbacks[i])();
    }
}
2 Answers

How about use a std::multimap? Key off the event type and use the handler for the value. When you want to issue an event use std::equal_range to get the elements that match your event type.

Note, however, that different events really should have different arguments. You could make the value be a std::any instead:

/* At class scope */
using resize_callback = void (*)(unsigned, unsigned);
using events_coll = std::multimap<event, std::any>;

events_coll events;

void add_resize(resize_callback pfn)
{
  events.insert(std::make_pair(WINDOW_REISZE, boost::any(pfn));
}

void issue_resize(unsigned width, unsigned height)
{
  std::pair<event_coll::iterator, event_coll::iterator> pr = events.find(WINDOW_RESIZE);
  for(std::multimap<event, std::any> it = pr.first; pr.second != it; ++it)
  {
    try
    {
      resize_callback pfn = it->as<resize_callback>();
      pfn(width, height);
    }
    catch(std::bad_any_cast &)
    {
      /* handle exception */
    }
  }
}

And have similar callback aliases and functions for your other event types.

This event system consists of a series of void pointers used for callback functions.

Use std::function in a suitable form / shape / wrapper.

Currently, the bad solution needs to know what vector to add to, and which vector to run based off of the event type.

Use a std::multimap from the event type enum to a suitable polymorphic handler type. With a polymorphic type you will need to reference it by (e.g.) a std::unique_ptr rather than directly embedding it in the multimap. Alternatively, a std::variant is something you could use (to avoid the extra pointer hops), but call sites may look slightly uglier. (OTOH, you don’t need std::visit or the like, because the multimap key already determines what type is expected.)

Ideally, I could store all the void pointers in a single vector, and based off of the type of event that occurs, the function pointers using that event could be run.

There is nothing ideal about this. It sounds like an unnecessary linear treversal through the common vector with all event types, only to run one event type. Bad idea.

Related