4

I want to implement a CommandBus that can Dispatch some Commands to CommandHandlers.

  • A Command is a simple a DTO describing what should happen. For instance : "Increment counter by 5"
  • A CommandHandler is able to handle a precise type of Command.
  • The CommandBus takes a Command and executes the CommandHandler that is able to handle it.

The code I wrote does not compile.

Compiler complains cannot convert from 'IncrementHandler' to 'Handler<Command>'. I don't understand why, because IncrementHandler implements Handler<Increment> and Increment implements Command

I've tried both in and out modifiers on the generic interfaces, it doesn't solve the problem.

Is there a way to achieve this with only interfaces ?

[TestClass]
public class CommandBusTest
{
  [TestMethod]
  public void DispatchesProperly()
  {
    var handler = new IncrementHandler(counter: 0);
    var bus = new CommandBus(handler); // <--Doesn't compile: cannot convert from 'IncrementHandler' to 'Handler<Command>'
    bus.Dispatch(new Increment(5));
    Assert.AreEqual(5, handler.Counter);
  }
}

public class CommandBus
{
  private readonly Dictionary<Type, Handler<Command>> handlers;

  public CommandBus(params Handler<Command>[] handlers)
  {
    this.handlers = handlers.ToDictionary(
      h => h.HandledCommand,
      h => h);
  }

  public void Dispatch(Command commande) { /*...*/ }
}

public interface Command { }

public interface Handler<TCommand> where TCommand : Command
{
  Type HandledCommand { get; }
  void Handle(TCommand command);
}

public class Increment : Command
{
  public Increment(int value) { Value = value; }

  public int Value { get; }
}

public class IncrementHandler : Handler<Increment>
{
  // Handler<Increment>
  public Type HandledCommand => typeof(Increment);
  public void Handle(Increment command)
  {
    Counter += command.Value;
  }
  // Handler<Increment>

  public int Counter { get; private set; }

  public IncrementHandler(int counter)
  {
    Counter = counter;
  }
}
BJ Myers
  • 6,617
  • 6
  • 34
  • 50
  • I don't understand why your are passing an array. Have you tried:public CommandBus(Handler handlers) { this.handlers = handlers.ToDictionary( h => h.HandledCommand, h => h); } – Kevin Raffay May 24 '17 at 16:33
  • I used `params` because I want to be able to pass as many Handlers as I want, like so : `new CommandBus(new AHander(), new BHandler(), new CHandler())` The bus will contain several Handlers, because there will be several types of Command. – Christophe Cadilhac May 24 '17 at 16:35
  • the `params` syntax is fine, it will appear as a `Handler[]` with one entry in the constructor. – Cee McSharpface May 24 '17 at 16:36
  • 1
    Given the definition of `Handler` you can only make it contravariant in `TCommand` but then `IncrementHandler` is not compatible with `Handler` since `Command` is a supertype of `Increment` and not a subtype as required. There's no way to do this safely so you'll have to cast somewhere. – Lee May 24 '17 at 16:37

3 Answers3

8

I don't understand why, because IncrementHandler implements Handler<Increment> and Increment implements Command

Let's fix your misunderstanding, and then the rest will become clear.

Suppose what you wanted to do was legal. What goes wrong?

IncrementHandler ih = whatever;
Handler<Command> h = ih; // This is illegal. Suppose it is legal.

now we make a class

public class Decrement : Command { ... }

And now we pass it to h:

Decrement d = new Decrement();
h.Handle(d);

This is legal, because Handler<Command>.Handle takes a Command, and a Decrement is a Command.

So what happened? You just passed a decrement command to ih, via h, but ih is an IncrementHandler that only knows how to handle increments.

Since that is nonsensical, something in here has to be illegal; which line would you like to be illegal? The C# team decided that the conversion is the thing that should be illegal.

More specifically:

Your program is using reflection in an attempted end-run around the type system's safety checks, and then you are complaining that the type system is stopping you when you write something unsafe. Why are you using generics at all?

Generics are (in part) to ensure type safety, and then you are doing a dispatch based on reflection. This doesn't make any sense; don't take steps to increase type safety and then do heroic efforts to work around them.

Plainly you wish to work around type safety, so don't use generics at all. Just make an ICommand interface and a Handler class that takes a command, and then have some mechanism for working out how to dispatch commands.

What I don't understand though is why there are two kinds of things at all. If you want to execute a command, then why not simply put the execution logic on the command object?

There are also other design patterns you could use here other than this clunky dictionary lookup based on types. For example:

  • a command handler could have a method that takes a command and returns a boolean, whether the handler can handle this command or not. Now you have a list of command handlers, a command comes in, and you just run down the list asking "are you my handler?" until you find one. If O(n) lookup is too slow, then build a MRU cache or memoize the result or some such thing, and the amortized behaviour will improve.

  • the dispatch logic could be put into the command handler itself. A command handler is given a command; it either executes it, or it recurses, calling its parent command handler. You can thus build a graph of command handlers that defer work to each other as necessary. (This is basically how QueryService works in COM.)

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • 1
    Typical - I use a couple hundred words to try to explain something, and then Eric Lippert comes along and explains it better in 5 lines of code. ;) – BJ Myers May 24 '17 at 17:34
  • Thanks Eric. At the time of writing I was thinking you might show up and answer :) You make good points on the possible enhancements of the pattern I'm using. This code was about implementing a version of CQRS that I was told about at a recent meetup. So I gave a go at the proposed design and faced the compilation issue. I think it's a smell that I should change something. I'll look into that! – Christophe Cadilhac May 29 '17 at 12:41
0

The problem here is that Increment implements Command (which I renamed to ICommand to make that clearer, in the code below). So it is no longer accepted as a Handler<Command>, which is what the constructor expects (subtype instead of required supertype, as @Lee pointed out in comments).

If you can generalize to use just ICommand, it would work:

public class CommandBusTest
{
    public void DispatchesProperly()
    {
        var handler = new IncrementHandler(counter: 0);
        var bus = new CommandBus((IHandler<ICommand>)handler); 
        bus.Dispatch(new Increment(5));
    }
}

public class CommandBus
{
    private readonly Dictionary<Type, IHandler<ICommand>> handlers;

    public CommandBus(params IHandler<ICommand>[] handlers)
    {
        this.handlers = handlers.ToDictionary(
          h => h.HandledCommand,
          h => h);
    }

    public void Dispatch(ICommand commande) { /*...*/ }
}

public interface ICommand { int Value { get; } }

public interface IHandler<TCommand> where TCommand : ICommand
{
    Type HandledCommand { get; }
    void Handle(TCommand command);
}

public class Increment : ICommand
{
    public Increment(int value) { Value = value; }

    public int Value { get; }
}

public class IncrementHandler : IHandler<ICommand>
{
    // Handler<ICommand>
    public Type HandledCommand => typeof(Increment);

    public void Handle(ICommand command)
    {
        Counter += command.Value;
    }

    // Handler<ICommand>

    public int Counter { get; private set; }

    public IncrementHandler(int counter)
    {
        Counter = counter;
    }
}
Cee McSharpface
  • 8,493
  • 3
  • 36
  • 77
0

The problem here is that your definition of Handler<TCommand> requires TCommand to be both covariant and contravariant - and that's not allowed.

To pass a Handler<Increment> into the constructor of CommandBus (which expects a Handler<Command>), you must declare Command as a covariant type parameter in Handler, like this:

public interface Handler<out TCommand> where TCommand : Command

Making this change allows you to pass in a Handler<AnythingThatImplementsCommand> wherever a Handler<Command> is requested, so your constructor for CommandBus works now.

But this introduces a new issue for the following line:

void Handle(TCommand command);

Since TCommand is covariant, it is possible to assign a Handler<Increment> to a Handler<Command> reference. Then you would be able to call the Handle method but pass in anything that implements Command - clearly that's not going to work. To make this call correct, you have to allow TCommand to be contravariant instead.

Since you can't do both, you'll have to make a concession somewhere. One way to do this is by making using covariance in Handler<TCommand>, but force an explicit cast in your Handle method, like this:

public interface Handler<out TCommand> where TCommand : Command
{
    Type HandledCommand { get; }
    void Handle(Command command);
}

public class IncrementHandler : Handler<Increment>
{
    public void Handle(Command command)
    {
        Counter += ((Increment)command).Value;
    }
}

It doesn't prevent somebody from creating an IncrementHandler and then passing in the wrong kind of Command, but if the handlers are only used by CommandBus you can check the type in CommandBus.Dispatch and have something resembling type safety.

BJ Myers
  • 6,617
  • 6
  • 34
  • 50