Generic message system using variadic templates

Question

in my spare time I´m creating a hobby game engine as decided to write some of my systems on my own. As the time passes I´ve decided to start publishing it as a independent libraries for use (so anybody can use it), the first one is my message system. The main task was simple : create a flexible system that allowed me to create a different types of the messages even during the runtime. For this I used the modern c++ features such as variadic templates, lambdas, smart pointers (to minimalize the memory leaks). I used an RTTI (typeid) because of the mechanism for detecting the given parameters when listening/binding to a message.

Some simple idea is :

1. Create a messages, save their hash(representing the variadic parameters) - via the createMessage function
2. Bind a functions to messages (when binding function, check the functions parameters if it fits the parameters that were given to created message) - via the bindFunction
3. Broadcast messages

For simplicity i defined it as a single-header library because of the ease of the compilation and portability. It is written in OOP, messages are created only via the itnerface from main class, MessageManager(static class at the moment) and listening+broadcasting is made via the objects called MessageHandlers.

    #pragma once

#include <vector>
#include <unordered_map>
#include <typeinfo>
#include <functional>
#include <utility>
#include <algorithm>
#include <memory>

namespace pin
{
    const std::string PIN_BLANK = "Blank";

    //Variadic function wrapper
    template<class... Args>
    class variadicFunction
    {
        private:
            //Functor
            std::function<void(Args...)> _function;

        public:
            variadicFunction() {};

            template<class T>
            void bindFunction(void(T::*function)(Args... args), T* ownerPtr)
            {
                //Saved via lambda to avoid generic placeholders with std::bind
                _function = [function, ownerPtr](Args... args)
                {
                    (ownerPtr->*function)(args...);
                };
            }

            void exec(Args&&... args)
            {
                if (_function)
                    _function(std::forward<Args>(args)...);
            }
    };

    //Base class for parameter pack so its able to combine it with polymorphism
    class base_param_pack{};

    //Variadic parameter pack
    template<class... Args>
    class param_pack{};

    class BaseMessage
    {
    protected:
        //Text of the message
        std::string _messageText;

        //Hash generated to check the type of the passed parameter pack
        std::size_t _parameterHash;

    protected:

        //Hashing function
        template <class... Args>
        void createHash()
        {
            //Create temporary parameter pack class from our passed arguments
            param_pack<Args...> tmpPack;

            //Save the ID of the pack using the RTTI
            _parameterHash = typeid(tmpPack).hash_code();

        }

    public:
        //Default constructor
        BaseMessage() :
            _messageText(PIN_BLANK),
            _parameterHash(0)
        {}

        //Constructing with the message name
        BaseMessage(const std::string& messageName) :
            _messageText(messageName),
            _parameterHash(0)
        {}

        virtual ~BaseMessage() {};

        virtual void clearListeners() = 0;

        //Checking the parameter hashes
        bool haveSameParameterHash(const std::size_t& paramHash)
        {
            return (_parameterHash == paramHash);
        }

        //Setting the message text
        void setMessageText(const std::string& text) { _messageText = text; }
        const std::string& getMessageText() { return _messageText; }
    };

    template <class... Args>
    class Message :public BaseMessage
    {
        typedef std::pair<bool*, variadicFunction<Args...>> listener;
    private:

        //Vector of listeners represented as variadic functions
        std::vector<listener> _listeners;

    protected:

        //Function checking if the listener exists or not
        bool isValidListener(const listener& listener)
        {
            return (listener.first);
        }

        void clearListeners() override
        {
            //Erase-remove idiom
            _listeners.erase(std::remove_if(
                                            _listeners.begin(),
                                            _listeners.end(),
                                            [](const listener& listener) 
                                            {
                                                return !*listener.first;
                                            }
                                            ),
                            _listeners.end());
        }

    public:

        Message():
        {
            createHash();
        }

        Message(const std::string& messageName) :
            BaseMessage(messageName)
        {
            createHash<Args...>();
        }

        //Broadcasting the message - no check if the parameter pack sets the hash
        void broadcast(Args&&... args)
        {
            for (auto& listener : _listeners)
            {
                listener.second.exec(std::forward<Args>(args)...);
            }
        }

        //Binding the function listener - no check if the parameter pack sets the hash
        template<class T, class... Args2>
        void bindFunction(bool* handler, void(T::*function)(Args2... args), T* ownerPtr)
        {
            _listeners.emplace_back();

            _listeners.back().first = handler;

            _listeners.back().second.bindFunction(function, ownerPtr);
        }
    };

    typedef std::unordered_map<std::string, std::unique_ptr<BaseMessage>> MessageMap;

    //Class that creates messages
    class MessageManager
    {
        friend class MessageHandler;

        private:
            static MessageMap _messages;

            /*
            *   Replace this with your own behaviour for giving bad hash 
            */
            static void incorrectHash()
            {
                std::cout << "pin::MessageSystem : The hash of given arguments do not fit" << std::endl;
            }

            /*
            *   Replace this with your own behaviour for giving bad message name
            */
            static void incorrectMessage()
            {
                std::cout << "pin::MessageSystem : Message not found" << std::endl;
            }

        protected:

            template<class T, class... Args>
            static BaseMessage* listenToMessage(bool* handler, const std::string& messageName, void(T::*function)(Args... args), T* ownerPtr)
            {
                //Check if the message with messageName exists
                auto it = _messages.find(messageName);

                if (it != _messages.end())
                {
                    //Create temporary hash
                    std::size_t tmpHash = typeid(param_pack<Args...>).hash_code();

                    //Check the created hash with the hash of the message
                    if (it->second->haveSameParameterHash(typeid(param_pack<Args...>).hash_code()))
                    {
                        //Bind the function
                        static_cast<Message<Args...>*>(it->second.get())->bindFunction(handler, function, ownerPtr);

                        return it->second.get();
                    }
                    else
                    {
                        /*
                        * User tried to pass a different parameter pack that was passed to message when you created it. Pass your own behavior here
                        */
                        incorrectHash();
                    }
                }
                else
                {
                    /*
                    * User tried to find a message that does not exists, put your own behaviour here
                    */
                    incorrectMessage();

                }

                return nullptr;
        }

        template<class... Args>
        static void broadcastMessage(const std::string& messageName, Args... args)
        {
            //Check if the message with messageName exists
            auto it = _messages.find(messageName);

            if (it != _messages.end())
            {
                //Generated hash for current parameter pack
                std::size_t tmpHash = typeid(param_pack<Args...>).hash_code();

                //Check if its the right hash
                if (it->second->haveSameParameterHash(typeid(param_pack<Args...>).hash_code()))
                {
                    //If its right broadcast the message
                    static_cast<Message<Args...>*>(it->second.get())->broadcast(std::forward<Args>(args)...);
                }
                else
                {
                    incorrectHash();
                }
            }
            else
            {
                incorrectMessage();
            }
        }

        public:

            template<class... Args>
            static void createMessage(const std::string& messageName)
            {
                _messages[messageName] = std::make_unique<Message<Args...>>(messageName);
            }
    };

    MessageMap MessageManager::_messages;

    //Class that can listen to messages or broadcast them
    class MessageHandler
    {
            private:
                bool _active;

                std::vector<BaseMessage*> _listenedMessages;
            public:

                MessageHandler():
                    _active(true){}

                ~MessageHandler()
                {
                    if (_active)
                        deleteHandler();
                }

                template<class T, class... Args>
                void listenToMessage(const std::string& messageName, void(T::*function)(Args... args), T* ownerPtr)
                {
                    //Vector of messages being listened to by this handler
                    _listenedMessages.emplace_back(MessageManager::listenToMessage(&_active, messageName, function, ownerPtr));
                }

                template<class... Args>
                void broadcastMessage(const std::string& messageName, Args... args)
                {
                    //Broadcast message
                    MessageManager::broadcastMessage(messageName, args...);
                }

                void deleteHandler()
                {
                    _active = false;

                    //All messages must clear their listeners - delete those who are inactive
                    for (auto& message : _listenedMessages)
                        message->clearListeners();
                }
    };
}

The pin prefix is just a prefix I use for my creations. The code is fully posted on github, also with some little tutorial for the use : github link

As I´m just a novice in the programming, I would like to hear some feedback from you guys. Especially the bad things, what should be improved. Thanks

Answer 1

Clearly define the use case

When I think of "message systems", it's either message passing between different computers, processes, or threads. However, it seems that your message system only works within a single thread of code. Also, it doesn't really store and forward messages, it merely registers and calls callback functions. This is fine, and may be exactly what you want, but make this absolutely clear for potential users of your message system.

Handles versus names

Your message system identifies message types by name. However, you have to programmatically register message types. Instead of using an unordered map to associate message types with names, why not have MessageManager::createMessage<>() return a reference to the Message object? This way, you can pass that reference to listenToMessage() and broadcastMessage(), instead of a name, and avoid doing map lookups every time.

Avoid global state

Your MessageMap is a global variable. What happens if you have a program that links with multiple libraries, each of which uses your message system? They will all use the same MessageMap. This might result in conflicts.

You already have made it so that you have to declare MessageManager variables before you can listen to and send Messages. Why not make MessageMap a non-static member variable of MessageManager?

Check for multiple registrations of the same name

Your code currently does not check if a program tries to call MessageManager::createMessage<>() twice with the same name. It will just overwrite the entry in the MessageMap with the second Message instance. Either avoid using names at all, or check that you don't add the same name twice (or alternatively, allow double registrations, but only as long as they all register the exact same message type).

hash_code() is not guaranteed to be unique

You are using hash_code() to check whether the type of a parameter pack is the same as that of a Message. However, while hash_code() guarantees that identical types get the same hash value, there is no guarantee that two different types get a different hash value.

If you avoid using message names, but rather have a reference to an actual Message object, then you could move functions like listenToMessage(), broadcastMessage() to the Message class. Then the compiler would exactly know the type of the parameter pack, without having to rely on run-time checks.

Use std::function<> for callbacks

Instead of forcing callbacks to be pointers to member functions, and requiring the user to specify both the member function and the object individually, use std::function<> to represent a callback function. The advantage is that it will accept any type of function as a callback, whether a named function, a lambda, or a member function (using std::bind). Make it like so:

template<class... Args>
void listenToMessage(const std::string& messageName, std::function<void(Args...)> callback)
{
    ...
}

To have a user specify a class member function as the callback, it has to be called like this:

Foo foo;
tut.listenToMessage("foo", std::bind(&Foo::test, foo));

But it makes it much easier to work with non-member functions or lambdas:

void non_member(int i) { std::cout << i; }
MessageManager::createMessage<int>("foo");
tut.listenToMessage("foo", non_member);
tut.listenToMessage("foo", [](int i){ std::cout << i; })