in

 

AgileJoe

Answering all world issues with, "...it depends..."

PTOM: The Open Closed Principle

The open closed principle is one of the oldest principles of Object Oriented Design. I won’t bore you with the history since you can find countless articles out on the net. But if you want a really comprehensive read please checkout Robert Martin’s excellent write up on the subject.

The open closed principle can be summoned up in the following statement.

open/closed principle states "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification";[1] that is, such an entity can allow its behavior to be modified without altering its source code.

Sounds easy enough but many developers seem to miss the mark on actually implementing this simple extensible approach. I don’t think it is a matter of skill set as much as I feel that they have never been taught how to approach applying OCP to class design.

A case study in OCP ignorance

Scenario: We need a way to filter products based off the color of the product.

All entities in a software development ecosystem behave a certain behavior that is dependent upon a governed context. In the scenario above you realize that you are going to need a Filter class that accepts a color and then filters all the products that have adhere to that color.

The filter classes’ responsibility is to filter products (its job) based off the action of filtering by color (its behavior). So your goal is to write a class that will always be able to filter products. (Work with me on this I am trying to get you into a mindset because that is all OCP truly is at its heart.)

To make this easier I like to tell developers to write the fill in the following template.

 

The {class} is responsible for {its job} by {action/behavior}

The ProductFilter is responsible for filtering products by color

 

Now let’s write our simple class to do this:

public class ProductFilter
{
    public IEnumerable<Product> ByColor(IList<Product> products, ProductColor productColor)
    {
        foreach (var product in products)
        {
            if (product.Color == productColor)
                yield return product;
        }
    }
}

As you can see this pretty much does the job of filtering a product based off of color. Pretty simple but imagine if you had the following typical conversation with one of your users.

 

User: “We need to also be able to filter by size.”

Developer: “Just size alone or color and size? “

User: “Umm probably both.”

Developer: “Great!”

So let’s use our OCP scenario template again.

The ProductFilter is responsible for filtering products by color

The ProductFilter is responsible for filtering products by size

The ProductFilter is responsible for filtering products by color and size

Now the code:

public class ProductFilter
{
    public IEnumerable<Product> ByColor(IList<Product> products, ProductColor productColor)
    {
        foreach (var product in products)
        {
            if (product.Color == productColor)
                yield return product;
        }
    }

    public IEnumerable<Product> ByColorAndSize(IList<Product> products, 
                                                ProductColor productColor, 
                                                ProductSize productSize)
    {
        foreach (var product in products)
        {
            if ((product.Color == productColor) && 
                (product.Size == productSize))
                yield return product;
        }
    }

    public IEnumerable<Product> BySize(IList<Product> products,
                                        ProductSize productSize)
    {
        foreach (var product in products)
        {
            if ((product.Size == productSize))
                yield return product;
        }
    }
}

This is great but this implementation is violating OCP.

Where'd we go wrong?

Let’s revisit again what Robert Martin has to say about OCP.

Robert Martin says modules that adhere to Open-Closed Principle have 2 primary attributes:

1. "Open For Extension" - It is possible to extend the behavior of the module as the requirements of the application change (i.e. change the behavior of the module).

2. "Closed For Modification" - Extending the behavior of the module does not result in the changing of the source code or binary code of the module itself.

Let’s ask the following question to insure we ARE violating OCP.

Every time a user asks for a new criteria to filter a product do we have to modify the ProductFilter class?
Yes! This means it is not CLOSED for modification.

Every time a user asks for a new criteria to filter a product can we extend the behavior of the ProductFilter class to support this new criteria without opening up the class file again and modifying it?
No! This means it is not OPEN for extension.

Solutions

One of the easiest ways to implement OCP is utilize a template or strategy pattern. If we still allow the Product filter to perform its job of invoking the filtering process we can put the responsibility of how the filtering is accomplished in another class by mixing in a little LSP to accomplish this.

Here is the template for the ProductFilterSpecification

public abstract class ProductFilterSpecification
{
    public IEnumerable<Product> Filter(IList<Product> products)
    {
        return ApplyFilter(products);
    }

    protected abstract IEnumerable<Product> ApplyFilter(IList<Product> products);
}

Let’s go ahead and create our first criteria, which is a color specification.

public class ColorFilterSpecification : ProductFilterSpecification
{
    private readonly ProductColor productColor;

    public ColorFilterSpecification(ProductColor productColor)
    {
        this.productColor = productColor;
    }

    protected override IEnumerable<Product> ApplyFilter(IList<Product> products)
    {
        foreach (var product in products)
        {
            if (product.Color == productColor)
                yield return product;
        }
    }
}

Now all we have to do is extend the actual ProductFilter class to accept our template ProductFilterSpecification.

public IEnumerable<Product> By(IList<Product> products, ProductFilterSpecification filterSpecification)
{
    return filterSpecification.Filter(products);
}

OCP goodness!

So lets make sure we are NOT violating OCP and ask the same questions we did before.

Every time a user asks for a new criteria to filter a product do we have to modify the ProductFilter class?
No! Because we have marshaled the behavior of filtering to the ProductFilterSpecification. "Closed for modification"

Every time a user asks for a new criteria to filter a product can we extend the behavior of the ProductFilter class to support this new criteria without opening up the class file again and modifying it?
Yes! All we simply have to do is pass in a new ProductFilterSpecification. "Open for extension"

Now let’s just make sure we haven’t modified too much from our intentions of the ProductFilter. All we simply have to do is validate that our ProductFilter still has the same behavior as before.

The ProductFilter is responsible for filtering products by color: Yes it still does that!

The ProductFilter is responsible for filtering products by size: Yes it still does that!

The ProductFilter is responsible for filtering products by color and size: Yes it still does that!

If you are a good TDD/BDD practitioner you should already have all these scenarios covered in your Test Suite.

Here is the final code:

namespace OCP_Example.Good
{
    public class ProductFilter
    {
        [Obsolete("This method is obsolete; use method 'By' with ProductFilterSpecification")]
        public IEnumerable<Product> ByColor(IList<Product> products, ProductColor productColor)
        {
            foreach (var product in products)
            {
                if (product.Color == productColor)
                    yield return product;
            }
        }

        [Obsolete("This method is obsolete; use method 'By' with ProductFilterSpecification")]
        public IEnumerable<Product> ByColorAndSize(IList<Product> products,
                                                    ProductColor productColor,
                                                    ProductSize productSize)
        {
            foreach (var product in products)
            {
                if ((product.Color == productColor) &&
                    (product.Size == productSize))
                    yield return product;
            }
        }

        [Obsolete("This method is obsolete; use method 'By' with ProductFilterSpecification")]
        public IEnumerable<Product> BySize(IList<Product> products,
                                            ProductSize productSize)
        {
            foreach (var product in products)
            {
                if ((product.Size == productSize))
                    yield return product;
            }
        }

        public IEnumerable<Product> By(IList<Product> products, ProductFilterSpecification filterSpecification)
        {
            return filterSpecification.Filter(products);
        }
    }

    public abstract class ProductFilterSpecification
    {
        public IEnumerable<Product> Filter(IList<Product> products)
        {
            return ApplyFilter(products);
        }

        protected abstract IEnumerable<Product> ApplyFilter(IList<Product> products);
    }

    public class ColorFilterSpecification : ProductFilterSpecification
    {
        private readonly ProductColor productColor;

        public ColorFilterSpecification(ProductColor productColor)
        {
            this.productColor = productColor;
        }

        protected override IEnumerable<Product> ApplyFilter(IList<Product> products)
        {
            foreach (var product in products)
            {
                if (product.Color == productColor)
                    yield return product;
            }
        }
    }

    public enum ProductColor
    {
        Blue,
        Yellow,
        Red,
        Gold,
        Brown
    }

    public enum ProductSize
    {
        Small, Medium, Large, ReallyBig
    }

    public class Product
    {
        public Product(ProductColor color)
        {
            this.Color = color;
        }

        public ProductColor Color { get; set; }

        public ProductSize Size { get; set; }
    }

    [Context]
    public class Filtering_by_color
    {
        private ProductFilter filterProduct;
        private IList<Product> products;

        [SetUp]
        public void before_each_spec()
        {
            filterProduct = new ProductFilter();
            products = BuildProducts();
        }

        private IList<Product> BuildProducts()
        {
            return new List<Product>
                               {
                                   new Product(ProductColor.Blue),
                                   new Product(ProductColor.Yellow),
                                   new Product(ProductColor.Yellow),
                                   new Product(ProductColor.Red),
                                   new Product(ProductColor.Blue)
                               };

        }


        [Specification]
        public void should_filter_by_the_color_given()
        {
            int foundCount = 0;
            foreach (var product in filterProduct.By(products, new ColorFilterSpecification(ProductColor.Blue)))
            {
                foundCount++;
            }

            Assert.That(foundCount, Is.EqualTo(2));
        }
    }
}
Published Mar 21 2008, 07:47 PM by Joe Ocampo
Filed under:

Comments

 

Jeremy Gray said:

On the other hand, you violated separation of concern by mixing Specification with your filtering, and overconstrained the filter mechanism by accepting IList and returning IEnumerable (whereas you should have accepted IEnumerable), so perhaps another post in the series is in order? :)

March 22, 2008 11:45 AM
 

Joe Ocampo said:

You know after I posted I thought the same thing.  But I was hoping no one would call me on it!  Thanks Jeremy! :-)

But yes I agree I should follow this up with another post.

March 22, 2008 12:23 PM
 

Dew Drop - March 22, 2008 | Alvin Ashcraft's Morning Dew said:

Pingback from  Dew Drop - March 22, 2008 | Alvin Ashcraft's Morning Dew

March 22, 2008 1:41 PM
 

Links Today (2008-03-23) said:

Pingback from  Links Today (2008-03-23)

March 23, 2008 10:17 AM
 

gOODiDEA said:

.NET:C#正则表达式整理备忘谈谈volatile变量Other:PTOM:TheOpenClosedPrincipleCalculatingpiwithC#...

March 23, 2008 8:56 PM
 

Yuanjian said:

.NET: C#正则表达式整理备忘 谈谈volatile变量 Other: PTOM: The Open Closed Principle Calculating pi with C# - Calculate

March 23, 2008 8:57 PM
 

Reflective Perspective - Chris Alcock » The Morning Brew #58 said:

Pingback from  Reflective Perspective - Chris Alcock  &raquo; The Morning Brew #58

March 25, 2008 2:17 AM
 

Whatever said:

Great stuff. I'm working hard to better understand OOD principles and work them in my code, and examples such as this help me to recognize when I'm doing something stupid.

Keep up the great work.

March 25, 2008 8:15 AM
 

DotNetKicks.com said:

You've been kicked (a good thing) - Trackback from DotNetKicks.com

March 25, 2008 2:49 PM
 

Christopher Bennage said:

Ok, I know this will be a bit much, but I just had a thought.  The quoted principle says OCP applies to "software entities". The scope of an entity in this example is a class, but could it be a whole application? Adding specification classes is modifying the code of the application. Where do we draw the line?

As a TDD'ers, we're inclined to do the simplest thing that works. If the client never mentioned size, then introducing specification might be YAGNI. Well, yeah, maybe...

I know I'm being a little silly, but I do think that in certain cases the principles can (seem to) be fuzzy and conflicting. Especially for newcomers. I'm always interested to know _why_ a given principle is useful.  The usual answer is because it promotes maintainability.

March 26, 2008 2:45 PM
 

Joe Ocampo said:

@Christopher

>I know I'm being a little silly, but I do think that in certain cases the principles can (seem to) be fuzzy and conflicting. Especially for newcomers.

Well it depends...  The principles are meant to guide you in creating better software. Maintainability is only one aspect of great software.  TDD promotes DRY and YAGNI mindsets but that doesn't take away from leveraging all the aspects of SOLID principles.

Remember in TDD the steps are Red, Green, Refactor. Most people are really good at the Red, Green part of that equation but leave out the refactor aspect.

One of the  reasons for people overlooking this is aspect is because it relies on experience and wisdom to guide you.  If you have never been introduced to a template or strategy pattern you will never know that you you need to refactor.  But if you have been trained and utilized these patterns you quickly see the opportunity to implement the pattern resulting in decreasing the technical debt through maintainability.  That is the beauty of patterns the a tried and true in decreasing complexity and increasing maintainability.

But like anything else these principle apply from a given context. As you mentioned the term "Entity" can be applied to any artifact of a software ecosystem.  At some point the artist must rely on experience and wisdom to guide their decisions.  I am not going to tell you that OCP will never be applicable at the assembly level but the probablity on its applicability is probably very small if not nil.

The principles are a result of good design pattern usage.  While the principle and patterns are mutually exclusive in their own right together they form a cohesive amalgam that leads to great software.

March 26, 2008 4:04 PM
 

{ null != Steve } » OCP Example said:

Pingback from  { null != Steve } &raquo; OCP Example

March 27, 2008 10:34 AM
 

Chad Myers' Blog said:

Pablo&#39;s Topic of the Month - March: SOLID Principles Over the next few days and weeks, the Los Techies

March 29, 2008 12:19 AM
 

AgileJoe said:

I was playing with some Ruby code this weekend and thought I would show some OCP with Ruby . For more

March 30, 2008 10:54 PM
 

GrabBag said:

The Dependency Inversion Principle, the last of the Uncle Bob &quot;SOLID&quot; object-oriented design

March 31, 2008 4:52 PM

Leave a Comment

(required)  
(optional)
(required)  

Enter the numbers above:
Add

About Joe Ocampo

I work as an Assistant Vice President & Architect for a large financial institution in San Antonio, TX. I have been working with .Net and various software development technologies for the past fourteen years. My background as a software developer has been a very interesting to say the least. I enlisted in the United States Air Force from October 93 to January 01. While in the Air Force I learned how to code in assembler, C++ and Visual Basic. Currently my language of choice is C# but I have been dabbling in more of the dynamic languages such as Ruby. I have taught various subjects in the IT arena ranging from A+, MCSE and MCSD courses at local Microsoft training facilities.

When I completed my second enlistment in the Air Force, I decided to return home to San Antonio. With my background in more formal processes such as Waterfall, RUP and Spiral Iterative development, I found it challenging to embrace Agile Disciplines. Then I had the privilege and opportunity to work with a great group of developers on an Agile project. My ignorance of Agile principles turned slowly into embracement. I quickly saw the benefits of XP, Scrum and {Insert agile process here}. Well you get the point. I am a Certified Scrum Master and pride myself on applying agile principles and practices in the enterprise arena. I view Agile development as a plethora of tools and practices that help to deliver software quickly, with quality and enormous business value.

In my spare time I love to program and gain knowledge of new ways on how to improve my craft. When I am not developing, I enjoy studying history and theology. I also have a loving passion for sweets! ;-)

Christopher Columbus sought out to prove that the earth was round in a time where the norm was that of a flat land, with boundaries and peril that awaited its border. Conventional wisdom at the time, challenged by innovation and above all courage. The rest as we all know is history. Prepare yourself for the journey ahead.

Thank you, Joe Ocampo "Ordo Ab Chao” Out of chaos comes order
Copyright Los Techies 2007. All rights reserved.
Powered by Community Server (Commercial Edition), by Telligent Systems