2
\$\begingroup\$

I wrote a simple concept of an event system in cpp. It is uses a namespace and a global state, but normally a global state with namespaces is bad practice.

Is there a better way to do simple event handling with multiple callback functions?

header:

#ifndef EVENTS_EVENTMANAGER_H
#define EVENTS_EVENTMANAGER_H
#include <glad/glad.h>
#include <GLFW/glfw3.h>
#include <vector>
#include <functional>

namespace eventmanager
{   

    //resize event
    static std::vector<std::function<void(GLFWwindow*, int, int)>> resizeEventCallbacks;
    void resizeEvent(GLFWwindow *window, int width, int hight);
    void subscribeResize(std::function<void(GLFWwindow*, int, int)> callback);
}

#endif //EVENTS_EVENTMANAGER_H

cpp:

#include "events/eventmanager.h"

void eventmanager::resizeEvent(GLFWwindow *window, int width, int hight)
{
    for (auto &callback: eventmanager::resizeEventCallbacks)
    {
        callback(window, width, hight);
    }
}

void eventmanager::subscribeResize(std::function<void(GLFWwindow *, int, int)> callback)
{
    eventmanager::resizeEventCallbacks.push_back(callback);
}

usage:

eventmanager::subscribeResize([&](GLFWwindow *window, int width, int hight) {
            ...

            });
\$\endgroup\$
0
\$\begingroup\$

I see one major issue:

static std::vector<std::function<void(GLFWwindow*, int, int)>> resizeEventCallbacks;

You should never put a static variable declaration like that in a header. That will cause every file that includes the header to have a local copy of that variable. What you probably want is to put that declaration in the .cpp file so that it is local to that file only. The resizeEventCallbacks is an implementation detail that should be known only to eventmanager.cpp functions.

\$\endgroup\$
  • \$\begingroup\$ Thank you. I didn't see that, you are totally right. Do you think the concept with the namespace is ok? \$\endgroup\$ – FirstOpenGLandCPP Dec 2 '17 at 17:27
  • \$\begingroup\$ I think it's fine. If you're going to end up dispatching a lot of different types of events, then it might make sense to have a class or set of classes to do it and keep it organized. But for the small number you've implemented so far, I think it's fine. \$\endgroup\$ – user1118321 Dec 2 '17 at 17:30

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.