Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

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?

share|improve this question
    
What's the point of this pattern? What do you gain over specifically typing Handle() so that it only takes a FooCommand? –  Bobson Jun 26 '13 at 21:19
    
@Bobson: This is useful in scenarios where you have many implementations of ICommandHandler for different commands. All those handlers are registered at a central location (the command dispatcher). It iterates over all registered commands and calls Handle only on those that CanHandle the current command. You can't implement something like this with generics, because you wouldn't be able to put them in a list. –  Daniel Hilgarth Jun 26 '13 at 21:22
    
What about declaring ICommandHandler<T> : ICommandHandler, storing as List<ICommandHandler> commands, and retrieving with commands.Where(x => x is ICommandHandler<FooCommand>)? –  Bobson Jun 26 '13 at 21:27
    
@DanielHilgarth This is something that C#'s unsophisticated type system handles really badly, in my opinion. You can't look up the handlers for a message without unsafe casts. It's frustrating because it seems like a very simple and common use case. (Languages with better type systems - those with type classes - can handle it in a type-safe way with fewer moving parts.) –  Benjamin Hodgson Apr 30 at 23:06

2 Answers 2

up vote 5 down vote accepted

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

share|improve this answer
    
Good point with the assertions, thanks. –  Daniel Hilgarth Jun 27 '13 at 11:31

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.

share|improve this answer
    
That isn't really the same. Your code assumes that the only functionality inside CanHandle is a type check. But this method really can do anything. For example, it could contain a security check or similar. That scenario couldn't be modeled with your approach. –  Daniel Hilgarth Jun 27 '13 at 8:57
    
@DanielHilgarth That's a valid point - I was just going off of the example you provided. However, you could reintroduce the CanHandle function as bool CanHandle(T command) and add a .Where(x => x.CanHandle(command)) to the Dispatch<T> call. You'd still preserve the type checking, and avoid the need to manually check for handling. –  Bobson Jun 27 '13 at 11:29
    
By re-introducing CanHandle we are back at square one: Should Handle assume that the client called CanHandle or not? –  Daniel Hilgarth Jun 27 '13 at 11:31
    
Not really - with this setup, CanHandle doesn't need to be exposed to the client at all. –  Bobson Jun 27 '13 at 13:07
    
CommandManager is the client. –  Daniel Hilgarth Jun 27 '13 at 13:09

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.