Tester/Doer pattern: Assume the caller conforms to the pattern or be defensive and repeat the check?

Question

Assume a simple class that implements the Tester/Doer pattern:

public class FooCommandHandler : ICommandHandler
{
    public bool CanHandle(object command)
    {
        return command is FooCommand;
    }

    public void Handle(object command)
    {
        var fooCommand = (FooCommand)command;
        // Do something with fooCommand
    }
}

Now, if someone doesn't conform to the pattern and calls Handle without verifying the command via CanHandle, the code in Handle throws an exception.

However, depending on the actual implementation of Handle this can be a whole range of different exceptions.

The following implementation would check CanHandle again in Handle and throw a descriptive exception:

public void Handle(object command)
{
    if(!CanHandle(command))
        throw new TesterDoerPatternUsageViolationException("Please call CanHandle first");

    // actual implementation of handling the command.
}

This has the advantage that the exception is very descriptive.
It has the disadvantage that CanHandle is called twice for "good" clients.

Is there a consensus on which variation should be used?

Answer 1

If you can't guarantee that Handle is always given something it can handle, then you have no choice but to repeat the check by calling CanHandle from Handle.

That doesn't mean you always have to incur the double check penalty. If you put the call to CanHandle in an Assert statement, then suitable compiler options can ensure that it is only run in debug builds and removed from release builds.

That way developers get slapped on the wrist when getting it wrong, while production code doesn't have to worry about the overhead. One caveat: you need to have a strong (unit) test suite in place to ensure that all calls to Handle are checked by the assertion.

---

Actually, it may be better to avoid the dilemma altogether.

The problem with a class with one method that validates X, and another method then processes X is that you make that class hugely dependent on its clients and you need to take steps to ensure that people use your class the way it was intended to be used.

If you make Handle itself responsible for returning whether the work is acceptable and will get done, then checking whether the work received is suitable becomes a natural part of how Handle should be implemented.

The contract of Handle now obviously includes refusing unsuitable work and any tests for implementers of the ICommandHandler interface would check that its Handle implementation would respond as desired both when handed suitable and when handed unsuitable work.

Using this approach you can do away with the CanHandle method from the interface entirely. CanHandle may still exist in a class implementing the ICommandHandler but if it does, it now is just an implementation detail of that class.

I have written up a slightly more extensive discussion of this here: Designing your way out of the CanHandle-Handle conundrum

Answer 2

I am of the opinion that there's no need for this specific pattern in C#, due to the features of the language. Here's how I'd implement it:

public class CommandManager
{
    private List<ICommandHandler> handlers = new List<ICommandHandler>();

    public void RegisterHandler(ICommandHandler handler) { handlers.Add(handler); }
    public void Dispatch<T>(T command)where T : ICommand { 
        foreach (var handler in handlers.OfType<ICommandHandler<T>>()) handler.Handle(command);
    }
}

public interface ICommand { }
public class FooCommand : ICommand { }
public class BarCommand : ICommand { }


public interface ICommandHandler { }
public interface ICommandHandler<T> : ICommandHandler where T : ICommand
{
    void Handle(T command);
}

public class FooCommandHandler : ICommandHandler<FooCommand>, ICommandHandler<BarCommand>
{
    public void Handle(FooCommand command)
    {
        Console.WriteLine("Called FooCommandHandler with FooCommand");
    }
        public void Handle(BarCommand command)
    {
        Console.WriteLine("Called FooCommandHandler with BarCommand");
    }
}

public class BarCommandHandler : ICommandHandler<BarCommand>
{

    public void Handle(BarCommand command)
    {
        Console.WriteLine("Called BarCommandHandler with BarCommand");
    }
}

Executing it would be like this:

void Main()
{
    var myCommands = new CommandManager();
    myCommands.RegisterHandler(new FooCommandHandler());
    myCommands.RegisterHandler(new BarCommandHandler());

    myCommands.Dispatch(new FooCommand());
    /* Output:
         Called FooCommandHandler with FooCommand
    */
    myCommands.Dispatch(new BarCommand());
    /* Output:
        Called FooCommandHandler with BarCommand
        Called BarCommandHandler with BarCommand
    */
}   

OfType<>() handles your checking for you - it will automatically filter to only the types which can handle the specified command and cast to that type so you can call Handle(). You add only a single Handle overload for each specific command it can handle, so you don't have to worry about it being called with an invalid command.