An Example of the Open/Closed Principle in Action

I saw someone on Twitter this month say that they’ve never really understood the Open/Closed Principle (OCP, the “O” in S.O.L.I.D.). I think it’s a very important concept in software architecture, but the terse statement maybe doesn’t make it too clear what it’s really about:

software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification

There are some other ways to interpret the Open/Closed Principle (the Wikipedia article about it talks about inheritance which I think is short sighted), but my restatement of the OCP would be:

“structure code such that additional functionality can be mostly written in all new code modules with no, or at least minimal, changes to existing code modules”

The key point is that it’s much less risky and usually easier to write brand new code — especially if the new code has minimal coupling to old code — than it is to modify existing code. Or to put it another way, can I continually add all new functionality to my existing system without causing a lot of regression bugs?

It’s not just risk either, it’s generally easier to understand complicated new code written on a blank canvas than it is to open up an existing code file and find the right places to insert your changes without breaking the old functionality.

 

Some examples of systems from my career that most definitely did not follow OCP might better illustrate why you’d care about OCP:

  1. Dynamic web page application that was effectively written in one single VB6 class. Every single addition or fix to the application meant editing that one single file, and very frequently broke existing functionality
  2. A large shipping application where every bit of routing logic for box positions within a factory floor were coded in a single, giant switch statement that shared a lot of global state. Again, changes to routing logic commonly broke existing functionality. The cost of regression testing this routing logic slowed down the team in charge of this system considerably.
  3. COBOL style batch processes coded in giant stored procedures with lots of global state
  4. Naive usages of Redux in Javascript could easily lead to the massive switch statement problem where all kinds of unrelated code changes involve the same central file

An OCP Example: Linq Provider Extensibility in Marten

We’ve been building (and building and building) Linq query support into Marten. Linq support is the type of problem I refer to as “Permutation Hell,” meaning that there’s an almost infinite supply of “what about querying by this type/operator/method call?” use cases. Recently, one of our early adopters asked for Linq support for querying by a range of values like this:

// Find all SuperUser documents where the role is "Admin", 
// "Supervisor", or "Director"
var users = theSession.Query<SuperUser>()
    .Where(x => x.Role.IsOneOf("Admin", "Supervisor", "Director"));

In the case above, IsOneOf() is a custom extension method in Marten that just means “the value of this property/field should be any of these values”.

I thought that was a great idea, but at the time the Linq provider code in Marten was effectively a “Spike-quality” blob of if/then branching logic. Extending the Linq support meant tracing through the largely procedural code to find the right spot to insert the new parsing logic. I think recognizing this, our early adopter also suggested making an extensibility point so that users and contributors could easily author and add new method parsing to the Linq provider.

What we really needed was a little bit of Open/Closed structuring so that additional method call parsing for things like IsOneOf() could be written in brand new code instead of trying to ram more branching logic into the older MartenExpressionParser class (the link is to an older version;)).

Looking through the old Linq parsing code, I realized there was an opportunity to abstract the responsibility for handling a call to a method in Linq queries behind this interface from Marten:

    /// <summary>
    /// Models the Sql generation for a method call
    /// in a Linq query. For example, map an expression like Where(x => x.Property.StartsWith("prefix"))
    /// to part of a Sql WHERE clause
    /// </summary>
    public interface IMethodCallParser
    {
        /// <summary>
        /// Can this parser create a Sql where clause
        /// from part of a Linq expression that calls
        /// a method
        /// </summary>
        /// <param name="expression"></param>
        /// <returns></returns>
        bool Matches(MethodCallExpression expression);

        /// <summary>
        /// Creates an IWhereFragment object that Marten
        /// uses to help construct the underlying Sql
        /// command
        /// </summary>
        /// <param name="mapping"></param>
        /// <param name="serializer"></param>
        /// <param name="expression"></param>
        /// <returns></returns>
        IWhereFragment Parse(
            IDocumentMapping mapping, 
            ISerializer serializer, 
            MethodCallExpression expression
            );
    }

The next step was to pull out strategy classes implementing this interface for the method we already supported like String.Contains()String.StartsWith(), or String.EndsWith(). Inside of the Linq provider support, the next step was to select the right strategy for a method expression and use that to help create the Sql string:

protected override Expression VisitMethodCall(MethodCallExpression expression)
{
    var parser = _parent._options.Linq.MethodCallParsers.FirstOrDefault(x => x.Matches(expression)) 
        ?? _parsers.FirstOrDefault(x => x.Matches(expression));

    if (parser != null)
    {
        var @where = parser.Parse(_mapping, _parent._serializer, expression);
        _register.Peek()(@where);

        return null;
    }


    throw new NotSupportedException($"Marten does not (yet) support Linq queries using the {expression.Method.DeclaringType.FullName}.{expression.Method.Name}() method");
}

Once that was in place, I could build out the IsOneOf() search functionality by building an all new class implementing that IMethodCallParser interface described above. To wire up the new strategy, it was a one line change to the existing Linq code:

        // The out of the box method call parsers
        private static readonly IList<IMethodCallParser> _parsers = new List<IMethodCallParser>
        {
            new StringContains(),
            new EnumerableContains(),
            new StringEndsWith(),
            new StringStartsWith(),

            // Added
            new IsOneOf()
        };

So yes, I did have to “open” up the existing code to make a small change to enable the new functionality, but at least it was a low impact change with minimal risk.

I didn’t show it in this post, but there is also a new way to add your own implementations of IMethodCallParser to a Marten document store. I’m not entirely sure how many folks will take advantage of that extensibility point, but the structural refactoring I did to enable this story should make it much easier for us to continue to refine our Linq support.

My example is yet another example of using plugin strategies to demonstrate the Open/Closed Principle, but I think the real emphasis should be on compositional designs. Even without formal plugin patterns or IoC containers or configuration strategies, using the OCP to guide your design thinking about how to minimize the risk of later changes is still valuable.

 

 

 

 

 

Advertisements

6 thoughts on “An Example of the Open/Closed Principle in Action

  1. Pingback: The Morning Brew - Chris Alcock » The Morning Brew #2048

  2. Pingback: Dew Drop – March 9, 2016 (#2204) | Morning Dew

  3. dotnetchris

    My goto example illustration of violation of the OCP are all of the classes that require you to call Init() style methods since this now requires all inheritors to have external knowledge

    Reply
  4. Pingback: Weekly Links #5 | Useful Links For Developers

  5. Pingback: Compelling Sunday – 20 Posts on Programming and QA

  6. Graham Hay

    I think it’s important to remember to only do this in places where it has become necessary though (i.e. the rule of 3). I’ve worked in too many codebases where everything was an extension point 🙂

    Reply

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s