The Open/Closed Principle – A real world example

Out of the five SOLID principles the Open/Closed Principle is probably the one that I’ve had the hardest time understanding. However, a while ago though I found some code that I had written years ago that made me think to myself “Hey, this is clearly violating the Open Closed Principle!”. I had been thinking of reusing this code in the project I was working on, this blog, but realized that I would have to rewrite it to abide by OCP to satisfy my ambition of writing a flexible and well designed application.

The case – protecting the blog from spam bots

The case was this: I was writing a (ASP.NET MVC) controller that handled adding new comments from visitors to a blog post. Naturally I wanted to stop spam bots from posting comments but I didn't want to force visitors to fill in some CAPTCHA text. So, I decided to check commenting visitors IP address’ against Project Honey Pot’s IP black list. While that strategy doesn’t give bulletproof protection against spam bots it’s very easy to implement so I thought I’d start with that and then add other protection, such as Akismet, later on should I need to.

As I had implemented spam bot protection with Project Honey Pot before I dug up that old piece of code. It consisted of a single method that was called when a new comment was about to be saved. My old implementation looked something like this:

public class EntryController
{
    public void AddComment()
    {
        if(ValidateNotSpam())
        {
             //Save the comment to the database
        }
    }
    
    private bool ValidateNotSpam(string comment)
    {
        //Check if the IP-address that made the request
        //is known as a spammer by Project Honey Pot
    }
}

This code has a few problems. First of all it is responsible for doing two things, validating the request and persisting the comment, which isn't optimal. It's dependency on an external service renders it almost impossible to test. It is also clearly violating the Open/Closed principle.
Let's say I wanted to extend it so that it also used another service for validating the comment. I would then have no other choice but to modify the ValidateNotSpam method. And if I where to ship the compiled code as a product the users of it would have no way to add another service. Hence, it's not possible to extend the class, that is it isn't open for extension, and to modify behavior that isn't at the core of what the class should do (persisting comments) requires the actual class to be changed, it isn't closed for modification.

A better solution – abiding by the Open Closed Principle

Instead of using that old class I decided to come up with a better solution, which (I think) abides by OCP and which is much easier to extend. First of all I created an interface called ICommentValidator with a single method.

public interface ICommentValidator
{
    bool Validate(string authorIP, string content);
}

I then let my controller require an array of instances of ICommentValidator as a constructor parameter.

public class EntryController : Controller 
{
    private ICommentValidator[] _commentValidators;

    public EntryController(ICommentValidator[] commentValidators)
    {
        _commentValidators = commentValidators;
    }
}

I also modified the ValidateNotSpam method so that it invoked the Validate method on all of the ICommentValidators supplied to the constructor.

public class EntryController
{
    private bool ValidateNotSpam(string comment)
    {
        foreach (ICommentValidator validator in _commentValidators)
        {
            if (!validator.Validate(Request.ServerVariables["REMOTE_ADDR"], comment))
                return false;
        }

        return true;
    }
}

Finally I created a new class, ProjectHoneyPotCommentValidator that implemented the ICommentValidator interface and supplied an array with an instance of that class to the EntryControllers constructor. Actually I didn’t do that last part myself though, but instead let StructureMap instantiate the EntryController with instances of all ICommentValidator that it could find in the application domain, but that’s sort of beside the point.

public class ProjectHoneyPotCommentValidator : ICommentValidator
{
    public bool Validate(string authorIP, string content)
    {
        //Check if authorIP is blacklisted with 
        //Project Honey Pot
    }
}

Conclusion

In order to satisfy the Open/Closed Principle I had to make quite a few changes and add some extra abstraction but as a result my EntryController class is now much more flexible while at the same time it’s more stable as it’s less likely that it will have to be changed in the future. Choosing this implementation might have cost me about 15 minutes extra development time, but it didn’t take me long until that paid off when I had to add more validation functionality which was now a breeze. I also think that this code is much easier to maintain.

Any feedback is much appreciated! Did this example make sense? Was it a good example of the Open Closed Principle? Have I perhaps still misunderstood the principle?