About Me

Training

Develop With Passion® - Developer Bootcamp

Subscribe

The Open Closed Principle. - How Far Do You Take It

Written April 10, 2007 at 11:42 MDT Tagged programming

A couple of good comments came up in the last post about the use of utility classes in the realm of SRP. I made the statement that I personally don't mind using utility classes sparingly, as long as the class itself is adhering to the rules of SRP, meaning that the methods it is composed of revolve around one discrete area of functionality.

Ayende brought up a good code example and asked Jeff how he would handle the situation without using utility classes (Jeff posted a response to this question). This is the code that Ayende provided:

public void QueueToExecute(ICommand command)
{

  Validation.NotNull(command, "command");

  // do stuff
}

For those not familiar, the check that is being performed to ensure that the command is not null could be viewed as a simple validation check. Some developers refer to this as more than simple validation checking and refer to it as DBC (Design By Contract). Intentional or not, the explicit checking of the 'non null precondition' is a DBC technique, used to indicate to the client developer preconditions that must be in place for the method to continue, I digress!!

So what does OCP have to do with any of the code that was talked about the other day? Looking at the code above, I could make an assumption that the method lives on some sort of ICommandQueue implementation (we'll call it CommandQueue). And it could look something like this (lots omitted for brevity):

public class CommandQueue : ICommandQueue
{
    private Queue<ICommand> commands;

    public CommandQueue() : this(new Queue<ICommand>())
    {
    }

    public CommandQueue(IEnumerable<ICommand> initialSetToQueue)
    {
        commands = new Queue<ICommand>(initialSetToQueue);
    }

    public void QueueToExecute(ICommand command)
    {
        commands.Enqueue(command);
    }
}

Notice that the current version of CommandQueue does not contain the check for nulls on the QueueToExecute method. I could leverage DBC to ensure that preconditions expected by the client are met before adding the Command to the queue. This results in the call to the utility Validation class that Ayende brought in earlier:

public void QueueToExecute(ICommand command)
{
    Validation.NotNull(command,"command");
    commands.Enqueue(command);
}

This works. If the Validation class is simply a static container for procedural methods the NotNull method could look as follows:

public class Validation
{
    public static  void NotNull(object item,string itemDescription)
    {
        if (item == null) throw new ContractViolationException(itemDescription);
    }
}

On the other hand, Validation could be a mere Gateway that provides convienient access to discrete Assertion objects etc. I could carry on but I would rather focus on the current consumer of the Validation class itself. The CommandQueue. In the original implementation of CommandQueue, it had no smarts about performing the null check on incoming commands. By introducing the explicit check in the QueueToExecute method I have satisfied a rule that needed to be encapsulated for the method, but in the process I have violated the Open Closed Principle.

The Open Closed Principle

'a class should be Open for extension but Closed for modification'.

Back to the CommandQueue example, the original CommandQueue implementation worked fine before the requirement came along that said that it should ensure that incoming commands are not null. If I wanted to adhere to OCP, the 'Closed' part of the principle states that I should not need to change my original implementation to deal with the new requirement. The 'Open' part of the principle means that my design should allow me to add new behaviour to a CommandQueue without needing to go in and actually change the code of the original CommandQueue implementation. There are a couple of ways I could accomplish this and still adhere to OCP. For now, I'll use a Proxy to ensure that commands coming in cannot be null:

public class NullRejectingCommandQueue : ICommandQueue
{
    private ICommandQueue queueToProxy;

    public NullRejectingCommandQueue(ICommandQueue queueToProxy)
    {
        this.queueToProxy = queueToProxy;
    }

    public void QueueToExecute(ICommand command)
    {
        Validation.NotNull(command,"command");
        queueToProxy.QueueToExecute(command);
    }

}

With this proxy in place, I can remove the validation check from the original CommandQueue, and I have extended it without changing it. In my service layer (or container preferably), I could ensure that all instances of CommandQueues get wrapped in the NullRejectingCommandQueue proxy so that the null check will be performed. Without bringing a container into the mix, I could use a factory to get instances of CommandQueues:

public class CommandQueueFactory : ICommandQueueFactory
{
    public ICommandQueue Create()
    {
        return new NullRejectingCommandQueue(new CommandQueue());
    }
}

Reality Check

I am hoping that most of you realize that this could very well be overkill for what you are trying to do. Part of evolving as a good designer is knowing when trying to adhere to a principle, or leverage a pattern can actually be detrimental to your code. What can help you answer that question? Time and practice. The first step in becoming a good designer, is being able to first identify the smells in your code. Just because you have a cold does not mean that everyone else can't smell the fish in your garbage can. Once you are able to identify the code smells, before even looking at design patterns, you should get a good handle on some of the fundamental design principles that can lead you to cleaner OO designs (SRP and OCP being 2 of them).

Remember how I said that all of the design principles in one way or another tie back to SRP. In this example, performing the check for null commands was not an intended 'responsibility' of the command queue class. It's responsibility was to queue commands for execution. By adding the null check, we added added another responsibility to the CommandQueue class. I am not suggesting that the null check should not be there. I just wanted to demonstrate how you can extend the behaviour of an object (open it up with new responsibilities) without having to change the target object itself.