Contract.Requires and the Decorator Pattern. How to avoid overchecking conditions?

Question

I currently have a command handling interface that is implemented by a few different classes for different command types. I'm using the Decorator Pattern in conjunction with an IoC container (Unity in my case) to add cross cutting concerns to those handlers, so I created a few classes like:

ValidatorCommandHandlerDecorator
LoggingCommandHandlerDecorator
AsyncCommandHandlerDecorator

This is all working as expected and is actually very nice. The potential problem here is that the code contracts for the interface is checked for every decorator implementation. I would like to potentially avoid that by only validating the contract once (preferably on the outermost decorator).

Is there something available out of the box to accomplish that? If not, what would be your suggestion to overcome this issue?

The generic interface and the contract class are like this:

[ContractClass(typeof(CommandHandlerContracts<>))]
public interface ICommandHandler<TCommand>
{
    void Handle(TCommand command);
}

[ContractClassFor(typeof(ICommandHandler<>))]
internal abstract CommandHandlerContracts<TCommand>
    : ICommandHandler<TCommand>
{
    public void Handle(TCommand command)
    {
        Contract.Requires<ArgumentNullException>(
            command != null);
    }
}

And the ValidatorCommandHandler (as an example of how I'm implementing them) looks like this:

public sealed class ValidatorCommandHandlerDecorator<TCommand>
    : ICommandHandler<TCommand>
{
    private ICommandHandler<TCommand> m_decoratedHandler;
    private ICommandValidator<TCommand> m_commandValidator;

    public ValidatorCommandHandlerDecorator(
        ICommandHandler<TCommand> decoratedHandler,
        ICommandValidator<TCommand> commandValidator)
    {
        m_decoratedHandler = decoratedHandler;
        m_commandValidator = commandValidator;
    }

    public void Handle(TCommand command)
    {
        m_commandValidator.Validate(command);
        m_decoratedHandler.Handle(command);
    }
}

Even if I created another interface to use just for the decorators, it would have to inherit the original interface and the contracts would still be checked, so I'm not sure how to go about this.

This is .NET 4.0 / VS2012 and I'm using the latest version of Code Contracts, if this helps any.

Steven · Answer 1 · 2014-09-15T17:30:43.210

You defined a contract on that interface, so Code Contracts will ensure the contract is met; a contract is a contract. I guess you worry about performance, but this shouldn't be any problem from a performance perspective, since I expect the code that Code Contracts weaves in would be very fast (just a null check) in this case. You can verify this by using a IL decompiler.

But if you really want to do this, the solution is actually really simple. When looking at what you're doing the question even seems a bit silly, since you should already know the answer:

Use a decorator!

Here is your new outer-most decorator:

public class ContractCheckerCommandHandlerDecorator<TCommand>
    : ICommandHandler<TCommand>
{
    private readonly ICommandHandler<TCommand> decoratee;

    public ContractCheckerCommandHandlerDecorator(
        ICommandHandler<TCommand> decoratee)
    {
        this.decoratee = decoratee;
    }

    public void Handle(TCommand command)
    {
        if (command == null)
        {
            throw new ArgumentNullException("command");
        }

        this.decoratee.Handle(command);
    }
}

Note however that when using Unity, the overhead of adding an extra decorator to do the null check, will be much greater than adding null checks to each and every implementation using Code Contracts. On most cases, the overhead would probably still be neglectable when adding this extra decorator using Unity.

Thanks a lot for the insight. I too suspect the performance hit is minimal, but still wondered if there was a way to accommodate this using something from the contracts library that I did not know about. Also, just to be clear here, your articles are what inspired me to pursuit this design, and I'm loving it thus far. I think I will keep things like they are now though. If any decorator implementation tries to be funny and passes null to the next one in the chain this could be problematic in the future. Will wait a bit to see if someone else has any other suggestions to mark this as the answer — julealgon, Apr 12 '13 at 19:05
Also, I see you edited the question for readability purposes. I noticed you removed the 'readonly' markers on both fields too. Is there a reason for that other than making the text shorter? I think it is actually good practice to lock away things as much as possible (like using sealed and readonly). — julealgon, Apr 12 '13 at 19:14
That's indeed just for readability purposes. Readonly is good practice. I tried to make the code readable for people that have increased the font size (like myself) — Steven, Apr 12 '13 at 21:42

score 2 · Answer 2 · answered Apr 12 '13 at 17:49

2

If the preoccupation is about the contracts being verified on every Handle(ICommand) called. I´d say that´s okay since that´s what´s expected, since there isn´t a guarantee of what the wrapper is going to pass to the wrap-pee in a decorator chain, even if receive a non-null command.

answered Apr 12 '13 at 17:49

Ahmed Alejo

2,334
2
16
16

Yeah, I think this is reason enough to keep the structure as it is now too. – julealgon Apr 12 '13 at 19:09

score 1 · Accepted Answer · answered Apr 16 '13 at 13:30

Once again I got a very interesting answer from Manuel Fahndrich in this thread over the Code Contracts devlabs forum.

There is an attribute that can be placed over members to setup specific contract behavior options, and this one is used to disable contract runtime checking on the handle method for instance:

[ContractOption("runtime", "checking", false)]
public void Handle(TCommand command)
{ ... }

As I said in the comments, I will keep the runtime checks in case some decorator decides do pass null to it's decorated class (completely possible). Since the overhead should be minimal, I ended up abandoning the initial idea. It is very nice to know that such a thing is possible though, and maybe this answer is useful to someone else when searching.

I also ran into this problem when I was having to check for `null` inside each of my decorators. In the end, I decided to do the `null` check in my dispatchers since all commands/queries in my current design require that they go though the dispatchers in order to have their decorator chains resolved. — George Howarth, Jun 23 '14 at 13:52

Contract.Requires and the Decorator Pattern. How to avoid overchecking conditions?

3 Answers3

Use a decorator!