Yet another command / event dispatcher

Question

I'm trying to implement some kind of command dispatcher. I've already developed some code, but I'm not sure about a few things.

My dispatcher is a simple command dispatcher, so for each command, there can be only one handler registered. Let's skip considerations about multithreaded issues as I believe that code could be also adapted to work in multithread env. I just wanted to keep it stupid simple.

There are two parts that smell a little bit and I would be grateful if you could point out what in your opinion could go wrong.

First, the part of code responsible for generating id for command handler. It has to be done in order to match the command with the appropriate handler. Second, the part of code where I static cast handler to more specific handler. I assumed that if the id generated from the command exists in the map, then it means that this particular handler can be cast to a more specific handler. I don't like casting too much but it's hard to find other solutions. I mean, there are other solutions, I've also developed one more but first I would like to focus on this one. However, if you would like to have a look at the other solution, here is a link to compiler explorer: click

I'm allowed to use GCC 11 and C++20.

//UPDATE I updated my question in order to show a context where I would like to use it. Actually, I was expecting that without a proper context it would be difficult to find a reason for using this solution. G. Sliepen suggested that the better would be to use the templated version of CommandDispatcher. However, there is one problem I see when using this approach. The problem is, that I can't just take the instance of templated CommandDispatcher (without specifying a concrete command template) and inject it in any place I need it. Actually, I could do that, but...it would require injecting exactly the specific template of CommandDispatcher. Maybe 'dispatcher' is not the best name for this class. Perhaps CommandBus would be better.

Let's consider the following example. Let's say that the control interface of my application is exposed as a REST http server. (it could be also mqqt client, tcp client, whatever, but http example is the easiest one to understand).

So, moving further let's assume that each resource (in my example hatch, and board resources, are controlled by separated controllers classes).

So for hatch resource there is something like HatchController class. It has CommandBus injected through the constructor.

- when HatchController receives POST requset on uri: myapp.com/hatch/open
then it calls commandBus.dispatch<OpenHatch>()

- when HatchController receivs POST request on uri: myapp.com/hatch/close
then it calls commandBus.dispatch<CloseHatch>();

The same applies for board resource.

- when BoardController receives POST request on uri: myapp.com/board/restart 
then it calls commandBus.dispatch<Restart>();

AS you can see each of the controllers shares exactly the same same commandBus (commandDispatcher).

If I were to use templated CommandDispatcher, then I would have to inject to e.g. HatchController, 3 or more (dependes on the numbers of supprted command) dispatchers, which in my opinion does not make sense.

#include <memory>
#include <iostream>
#include <functional>

struct OpenCover {};
struct CloseCover {};
struct Restart {};

size_t base_id;

template<class T>
size_t generate_command_id()
{
  static size_t id = base_id++;

  return id;
}

class CommandHandler
{
public:
  virtual ~CommandHandler() = default;
};

template<class Command>
class GenericCommandHandler : public CommandHandler
{
public:
  template<class Handler>
  explicit GenericCommandHandler(Handler&& handler)
    : handler_{std::forward<Handler>(handler)}
  {}

  void dispatch(const Command& command)
  {
    handler_(command);
  }

  std::function<void(const Command&)> handler_;
};

class CommandDispatcher
{
public:
  template<class Command, class Handler>
  void register_handler(Handler&& handler)
  {
    const auto command_id = get_command_id<Command>();

    if (handlers_.contains(command_id)) {
      throw std::runtime_error("Command handler has been already registred");
    }

    handlers_[command_id] = std::make_unique<
      GenericCommandHandler<Command>>(std::forward<Handler>(handler));
  }
  template<class Command>
  void dispatch(Command&& command)
  {
    const auto command_id = get_command_id<Command>();

    if (not handlers_.contains(command_id)) {
      throw std::runtime_error("Handler not found");
    }

    auto& base_handler = handlers_[command_id];
    auto& handler = static_cast<GenericCommandHandler<Command>&>(*base_handler);
    handler.dispatch(std::forward<Command>(command));
  }
private:
  template<class Command>
  size_t get_command_id()
  {
    return generate_command_id<std::decay_t<Command>>();
  }

private:
  std::unordered_map<size_t, std::unique_ptr<CommandHandler>> handlers_;
};

int main()
{
  auto dispatcher = CommandDispatcher{};
  dispatcher.register_handler<OpenCover>([](const auto& cmd) {
    std::cout << "OpenCover\n";
  });
  dispatcher.register_handler<CloseCover>([](const auto& cmd) {
    std::cout << "CloseCover\n";
  });
  dispatcher.register_handler<Restart>([](const auto& cmd) {
    std::cout << "Restart\n";
  });//  

  dispatcher.dispatch(OpenCover{});
  dispatcher.dispatch(CloseCover{});
  dispatcher.dispatch(Restart{});
}

Here is also Compiler Explorer link: click

Answer 1

Consider making CommandDispatcher itself a template

A lot of trouble in your code comes from the fact that CommandDispatcher itself is not a template, but its functions are. And to type-erase the handler function and still be able to look it up by type requires you to generate IDs and use std::unique_ptrs. Instead of storing multiple handler types in one CommandDispatcher, consider making CommandDispatcher templated on the Command type, then it only has to deal with one handler of which it already knows the type.

Going further with this, most of your code is just a bunch of wrappers around std::function. It could almost be replaced with this:

#include <functional>
#include <iostream>

template<typename Command>
std::function<void(const Command&)> CommandDispatcher;

int main() {
    struct OpenCover{};
    struct CloseCover{};
    struct Restart{};

    // Adding dispatchers
    CommandDispatcher<OpenCover> =  [](const auto& cmd){ std::cout << "OpenCover\n"; };
    CommandDispatcher<CloseCover> = [](const auto& cmd){ std::cout << "CloseCover\n"; };
    CommandDispatcher<Restart> =    [](const auto& cmd){ std::cout << "Restart\n"; };
    
    // Dispatching commands
    CommandDispatcher<OpenCover>({});
    CommandDispatcher<CloseCover>({});
    CommandDispatcher<Restart>({});
}

The only issue with the above is that your code doesn't allow the handler to be changed after it has been registered, whereas this example allows you to change the handler function at any time. If you do not want that, you could make a wrapper for std::function that only allows setting it once.