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, no registration required.

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
add comment

2 Answers

up vote 3 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.

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

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
add comment

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.