2
\$\begingroup\$

I have this question asked as home assignment for an interview. I submitted my solution and did not get selected. Wanted to know your feedback for the solution.

Question:

Write a very simple event bus system in C++. You can use whatever version you are comfortable with.

For this system to operate you will need two functions. (How they are grouped etc is a challenge left up to you)

  • A function that can take an event name. For example "email" and a function that can be executed later.

  • A function that can trigger any stored events. For example when we trigger the "email" event any stored events where we called the above adding function would be invoked. in other words in pseudo code: add("email", executeMe) then invoke("email") I would expect executeMe to be called once. However

    add("email", executeMe) add("email1", dontExecuteMe) add("email", executeMe) invoke("email")

I would expect executeMe to be called twice, while dontExecuteMe shouldn't be called. invoke("email1") would obviously be the inverse. Once you have this working, please extend the system to allow the event functions to take a string argument. For example:

add("email", executeMe)
invoke("email", "our email to be sent");

When invoke is called executeMe should be called with "our email to be sent" as its argument. As a final stage, please extend the system to allow events to be cancelled. For example:

add("email", executeMe)
add("email", dontExecuteMe)
add("email", dontExecuteMe2)
invoke("email", "our email to be sent");

In this example, only executeMe should be run and it should cancel any other events for "email". All other subscribed events should not be invoked.

The existing functionality we have built should still work For example, if executeMe didn't cancel the other events. This would also be a valid use of the same system.

add("email", executeMe);                 
add("email", executeMe2);                 
add("email", executeMe3);  
invoke("email", "our email to be sent")

All three functions (executeMe, executeMe2, executeMe3) should still execute. All API design, data structure choices are left up to you. It should be assumed that this system will continue to need to have features added.

Event manager description:

  1. Use Event class to define events
  2. class EventHandler is an abstract class which wraps the handler methods.
  3. class ExecuteEventHandler is inherited from EventHandler.
  4. EventManager brings the Event and EventHandler together and provides following interface method

    a) Add (locks before adding) with a flag with default param indicating whether the handler should be executed

    b) Invoke (locks before invoking).

  5. class Utility added to log. Log method writes to the console.

Assumption:

  1. Defined Event and EventHandler as objects instead of string and function pointers respectively.
  2. Cleanup only those handlers with execute handler flag disabled.
  3. Do nothing if Invoke is called with an unregistered event.
#include <algorithm>
#include <mutex>
#include <string>
#include <unordered_map>
#include <vector>
#include <iostream>
using namespace std;

class Utility
{
public:
    static void Log(const std::string& message)
    {
        std::cout << message << std::endl;
    }
};

class Event
{
    std::string name;
public:
    Event(const std::string& name) :name{ name }
    {
    }

    ~Event()
    {
    }
    const std::string& GetName() const
    {
        return name;
    }

    bool operator==(const Event & ev)
    {
        return this->name == ev.name;
    }

};

class EventHandler
{
public:
    EventHandler() {};
    virtual ~EventHandler() {};

    virtual void Execute() = 0;
    virtual void Execute(const std::string& str) = 0;

};

class ExecuteEventHandler : public EventHandler
{
public:

    ExecuteEventHandler() : EventHandler()
    {
    }
    ~ExecuteEventHandler()
    {
    }
    void Execute()
    {
        std::string msg = "ExecuteEventHandler::execute called";
        Utility::Log(msg);
    }

    void Execute(const std::string& str)
    {
        if (str.size() == 0)
            Execute();
        else
        {
            std::string msg = "ExecuteEventHandler::execute called with argument: " + str;
            Utility::Log(msg);
        }
    }

};
std::mutex gMu;

class EventManager
{
    std::unordered_map<Event*, std::vector<std::pair<EventHandler*, bool>>> eventAndHandler;
    void Remove(EventHandler* eh) {};

public:

    EventManager()
    {
    }

    ~EventManager()
    {
    }

    //Adds an Event with its corresponding EventHandler
    //bool execute indicates whether the handler should be executed
    //If the event exists just adds the EventHandler
    //Else creates a new entry

    void Add(Event* ev, EventHandler* eh, bool execute=true)
    {
        std::lock_guard<std::mutex> lock(gMu);
        //auto it = find_if(eventAndHandler.begin(), eventAndHandler.end(), [&](auto& e) 
        //{
        //  return e.first.Name == ev.Name;
        //});
        auto it = eventAndHandler.begin();
        for (; it != eventAndHandler.end(); ++it)
        {
            if (*it->first == *ev)
                break;
        }
        std::string msg;
        if (it != eventAndHandler.end())
        {
            it->second.push_back(std::make_pair(eh, execute));
            msg = "Adding an EventHandler to an existing event [" + it->first->GetName() + "]";
        }
        else
        {
            eventAndHandler[ev].push_back(std::make_pair(eh, execute));
            msg = "Adding a new event [" + ev->GetName() + "] with its EventHandler";
        }
        Utility::Log(msg);
    }

    //Invokes the EventHandlers for the given Event.
    //If the Event is not registered, does notthing
    //Also removes EventHanlders with disabled execution
    void Invoke(Event* ev, const std::string & arg = "")
    {
        std::lock_guard<std::mutex> lock(gMu);
        auto it = eventAndHandler.begin();
        for (; it != eventAndHandler.end(); ++it)
        {
            if (it->first == ev)
                break;
        }
        std::string msg;
        if (it != eventAndHandler.end())
        {
            msg = "Executing handlers for event [" + it->first->GetName() + "]:";
            Utility::Log(msg);
            for (auto iteh = it->second.begin(); iteh != it->second.end(); )
            {
                if ((*iteh).second)//check if the handler should be executed :)
                {
                    (*iteh).first->Execute(arg);
                    ++iteh;
                }
                else
                {
                    iteh = it->second.erase(iteh); //Remove the dont execute handlers
                    msg = "Removing a do not execute handler for event [" + it->first->GetName() + "]:";
                    Utility::Log(msg);
                    if (iteh == it->second.end())
                        break;
                }
            }
        }
        else
        {
            msg = "Event [" + ev->GetName() + "] is not registered";
            Utility::Log(msg);
        }
    }

};

int main()
{

    EventManager evMgr;
    Event evEmail{ "email" }, evEmail2{ "email2" }, evEmail3{ "email3" };
    ExecuteEventHandler execute, execute2, execute3, execute4;

    ExecuteEventHandler dontExecute, dontExecute2;

    evMgr.Add(&evEmail, &execute);
    evMgr.Add(&evEmail, &dontExecute, false); //false indicates do not execute the handler
    evMgr.Add(&evEmail, &dontExecute2, false);
    evMgr.Add(&evEmail, &execute2);

    //Invoking email without argument
    evMgr.Invoke(&evEmail);

    //Invoking email with argument
    evMgr.Invoke(&evEmail, "this is email content");


    evMgr.Add(&evEmail2, &execute4);
    evMgr.Add(&evEmail2, &dontExecute2); //Registering dontExecute2 with email2 and it will be executed

    evMgr.Invoke(&evEmail2);

    //Invoking not registered event email3
    evMgr.Invoke(&evEmail3);

}
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. Code Review is not the site to ask for help in fixing or changing what your code does. Once the code does what you want, we would love to help you do the same thing in a cleaner way! Please see our help center for more information. \$\endgroup\$ – Edward Jun 10 at 22:39
  • 3
    \$\begingroup\$ Assuming that this is a working solution, please edit the code to add the relevant #include statements, so that we can run it. \$\endgroup\$ – 200_success Jun 10 at 22:48
  • 1
    \$\begingroup\$ Hi Edward, This post is to request in design / implementation improvements on working code. Hi 200_success, Added the include statements. Please let me know if it compiles. \$\endgroup\$ – ksh Jun 10 at 23:43
  • \$\begingroup\$ Does the c++11 tag here mean that you chose C++11 as your C++ version and improvements in future versions don't matter for the review? \$\endgroup\$ – nwp Jun 11 at 12:34
  • \$\begingroup\$ Yes, improvements in higher versions definitely matters. Is there a tag for c++11 or higher then I would use that. Thanks \$\endgroup\$ – ksh Jun 11 at 15:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.