Be cautious when implementing the Open Closed Principle

Caution

Without a clear understanding of polymorphism and composition, OCP and the rest of the principles of SOLID won't make much sense.

Overview Of OCP

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

What does this mean?

It means that your code should be written in a way that leaves it flexible to adapt to changes (Open), without modifying the code already written (Closed).

A simple use case: Filtering Customers by criteria

We have a user story in our application backlog which demand to us to retrieve all the required information about our customers stored in the database, based on different criteria.
So, let’s start with that:

    public enum CustomerType
    {
        UnRanked,
        Silver,
        Glod,
        Platinium
    }

And we have the Customer domain class

    public class Customer
    {
        public string Name { get; set; }
        public string LastName { get; set; }
        public string Address { get; set; }
        public CustomerType Type { get; set; }
    }

Now, we need to implement our filtering functionality. For example, we want to filter by Customer types:

    public class CustomersFilter
    {
        public List<Customer> FilterByType(IEnumerable<Customer> customers, CustomerType type) =>
                customers.Where(m => m.Type == type).ToList();
    }

public List<Customer> GetSilverCustomers()
{
    var customers = new List<Customer>
    {
       new Customer { Name = "C1", Type = CustomerType.Silver},
       new Customer { Name = "C2", Type = CustomerType.Silver},
       new Customer { Name = "C3", Type = CustomerType.Glod}
    };
    CustomersFilter customersFilter = new CustomersFilter();
    var result = customersFilter.FilterByType(customers, CustomerType.Silver);

    return result;
}

The user story is ok, validated and the code goes on production. After a couple of days, we receive a new user story in which the product owner wants to have a filter by subscription as well in the application.

What should we do in this case?

It should be easy, right?
We are going to add a second filter method by subscription and it will work perfectly.

Let's add the subscription enum

    public enum SubscribtionType
    {
        Free,
        Basic,
        Pro,
        Enterprise
    }

And now, we can add our new method in the CustomerFilter class

public class CustomersFilter
{
    public List<Customer> FilterByType(IEnumerable<Customer> customers, CustomerType type) =>
            customers.Where(m => m.Type == type).ToList();

    public List<Customer> FilterByScreen(IEnumerable<Customer> customers, SubscribtionType subscribtion) =>
        customers.Where(m => m.Subscribtion == subscribtion).ToList();
}

By testing the code that we have just added, everything works as it should, on the other hand we have just violated the OCP principle here because we have modified in the class instead of extending it.

Another problem here is that once a new filter is going to be expressed as a need, another filter method must be added.

So, what should we do in such cases?

There are several possible solutions, so that our code respects the OCP principle.
Solution 1: Use inheritance or interface implementation.
In the code below we are using interface implementation.

public interface IFilter<T, E> where T : class where E : Enum
{
    List<T> Apply(IEnumerable<T> collection, E type);
}

public class FilterBySubscibtion : IFilter<Customer, SubscribtionType>
{
    public List<Customer> Apply(IEnumerable<Customer> collection, SubscribtionType type)
    {
        return collection.Where(m => m.Subscribtion == type).ToList();
    }
}

public class FilterByCustomerType : IFilter<Customer, CustomerType>
{
    public List<Customer> Apply(IEnumerable<Customer> collection, CustomerType type)
    {
        return collection.Where(m => m.Type == type).ToList();
    }
}

And our calls will be like:

private List<Customer> customers { get; set; } = new List<Customer>
{
    new Customer { Name = "C1", Type = CustomerType.Silver, Subscribtion = SubscribtionType.Free},
    new Customer { Name = "C2", Type = CustomerType.Silver , Subscribtion = SubscribtionType.Basic},
    new Customer { Name = "C3", Type = CustomerType.Glod, Subscribtion = SubscribtionType.Basic}
};
public List<Customer> FilterCustomers(CustomerType customerType)
{
    IFilter<Customer, CustomerType> customersFilter = new FilterByCustomerType();
    var result = customersFilter.Apply(customers, customerType);
    return result;
}

public List<Customer> FilterCustomers(SubscribtionType subscribtion)
{
    IFilter<Customer, SubscribtionType> customersFilter = new FilterBySubscibtion();
    var result = customersFilter.Apply(customers, subscribtion);
    return result;
}

public List<Customer> GetSilverCustomers() => FilterCustomers(CustomerType.Silver);

public List<Customer> GetCustomersWithFreeSubscibtion() => FilterCustomers(SubscribtionType.Free);

As you can see in the code, we have set up two different implementations per filter type (subscription and customer type).
We also have two methods that call these two filters but we have redundancy in the code.

To avoid this kind of redundance, we will try to make a second implementation based on the specification design pattern.

Solution 2: Use specification pattern

In computer programming, the specification pattern is a particular
software design pattern, whereby business rules can be recombined by
chaining the business rules together using boolean logic. The pattern
is frequently used in the context of domain-driven design.

Source: Wikipedia - Specification pattern

The specification design model allows us to check if an object meets certain requirements (usually functional)
So, with this design pattern, we can reuse specs and combine them when needed.

Let's create our specifications:

public interface ISpecification<T>
{
    bool IsSatisfied(T item);
}

public class CustomerTypeSpecification : ISpecification<Customer>
{
    private readonly CustomerType _type;
    public CustomerTypeSpecification(CustomerType type)
    {
        _type = type;
    }
    public bool IsSatisfied(Customer item) => item.Type == _type;
}

public class SubscribtionTypeSpecification : ISpecification<Customer>
{
    private readonly SubscribtionType _type;
    public SubscribtionTypeSpecification(SubscribtionType type)
    {
        _type = type;
    }
    public bool IsSatisfied(Customer item) => item.Subscribtion == _type;
}

With the ISpecification interface, we can determine whether or not our criteria are met, and then we can send it to the Apply method from the IFilter interface.

Now, time to update our existing code to use these specifications:

    public interface IFilter<T> where T : class
    {
        List<T> Apply(IEnumerable<T> collection, ISpecification<T> specification);
    }

    public class CustomersFilter : IFilter<Customer>
    {
        public List<Customer> Apply(IEnumerable<Customer> collection, ISpecification<Customer> specification)
        {
            return collection.Where(m => specification.IsSatisfied(m)).ToList();
        }
    }

After updating the filter class, let's go to our finale class that use it and make some updates.

public class FilterService
{
    private readonly IFilter<Customer> _filter;

    private List<Customer> customers { get; set; } = new List<Customer>
    {
        new Customer { Name = "C1", Type = CustomerType.Silver, Subscribtion = SubscribtionType.Free},
        new Customer { Name = "C2", Type = CustomerType.Silver , Subscribtion = SubscribtionType.Basic},
        new Customer { Name = "C3", Type = CustomerType.Glod, Subscribtion = SubscribtionType.Basic}
    };
    public FilterService(IFilter<Customer> customerFilter)
    {
        _filter = customerFilter;
    }
    public List<Customer> FilterCustomers(ISpecification<Customer> specification)
    {
        return _filter.Apply(customers, specification);
    }

    public List<Customer> GetSilverCustomers() => FilterCustomers(new CustomerTypeSpecification(CustomerType.Silver));

    public List<Customer> GetCustomersWithFreeSubscibtion() => FilterCustomers(new SubscribtionTypeSpecification(SubscribtionType.Free));
}

As we can see we don’t have any code redundancy in our FilterService class.

Conclusion

We have just seen how OCP can help us create clean and maintainable code. But we must be careful when implementing this principle.

As we saw in our example, this wasn’t an efficient way to use class inheritance. But we shouldn't be afraid to do so, it's completely normal, but we should at least make these changes as discrete as possible.

In other cases, we will be led to use composition over inheritance and use a design pattern such as Strategy to achieve our goal.

To conclude, the idea of this article is to keep OCP in mind during our development and write extensible code as much as possible, because this leads to a maintainable, scalable and testable code base however how the implementation used.

Some useful links:
https://ardalis.github.io/Specification/
https://deviq.com/principles/open-closed-principle

Comments

[peerreynders]

1. we want to filter by Customer types

2. the product owner wants to have a filter by subscription as well

What is the motivation at (1) to introduce the ISpecification<T> concept?

There isn't any.

Introducing it at (1) requires prescience that (2) will be required in the future. However time and time again developers have been shown to be terrible at predicting future changes, often increasing complexity to gain generality that never pays off or worse gets in the way of change that is needed later.

The more realistic scenario is that the simple version is implemented at (1) and that ISpecification<T> is introduced at (2) - which means that nothing was ever "closed to modification".

Kevlin Henney's rant about the Open-Closed Principle.

[Khalid BOURZAYQ]

The idea behind this article is that we should write clean code from the beginning and the scenario i described was just to introduce the problem and show that the first code written was not good enough to follow the design principle.
If we talk about the efficiency of the SOLID principles, we have pros and cons but for me the pros are more than cons.
Thanks for your comment.

[peerreynders]

show that the first code written was not good enough to follow the design principle.

The final code only makes sense when both requirements (1, 2) are known.

Introducing ISpecification<T> when only (1) is known is a guess. It may be an educated guess but the investment doesn't really pay off until (2) materializes - before that the complexity it introduces is non-essential.

Compare that to

Kent Beck

@kentbeck

for each desired change, make the change easy (warning: this may be hard), then make the easy change

23:07 PM - 25 Sep 2012

It acknowledges that earlier we may have had insufficient information to create the optimal design (to grant the freedom to be changed in the way needed) but now that we know what needs to change, we adapt to reflect our new state of knowledge.

Our version 1 "state of knowledge" simply leads to

List<Customer> FilterByType(IEnumerable<Customer> customers, CustomerType type)

And version 2 might simply be

List<Customer> FilterByType(IEnumerable<Customer> customers, CustomerType type)

List<Customer> FilterBySubscription(IEnumerable<Customer> customers, SubscriptionType subscription)

At this point we may simply decide that duplication is preferable to the wrong abstraction and just leave it.

If we do decide to go ahead with ISpecification<T> because some followup conversations with the stakeholders strongly suggested that there may be other filter types needed later but some code we don't have access to already uses FilterByType we may even end up with

List<Customer> FilterByType(IEnumerable<Customer> customers, CustomerType type)

List<Customer> Apply(IEnumerable<Customer> collection, ISpecification<Customer> specification)

So the real reason to be cautious with the Open-Closed Principle is when it requires you to predict the future - because when you predict the future you'll likely be wrong (YAGNI).

"Be careful when choosing the areas of code that need to be extended; applying the Open-Closed Principle EVERYWHERE is wasteful, unnecessary, and can lead to complex, hard to understand code."
Head First Design Patterns; Eric Freeman & Elisabeth Freeman, 2004; p.87

Robert C. Martin wrote about OCP again in 2014 - essentially advocating a Plugin Architecture. When you design a plugin architecture you have already decided up front along which lines your system needs to be extensible (supported hopefully by good evidence of that necessity/benefit). So OCP is presented as a principle for systems design - not class design.

Also note that ISpecification<T> has exactly one method. As Scott Wlaschin observes function types take SRP and ISP to the extreme.

So given what C# is capable of nowadays

public class CustomerPredicates 
{
    public static bool IsGold(Customer c)
    {
        return c.Type == CustomerType.Gold;
    }

    public static bool IsSilver(Customer c)
    {
        return c.Type == CustomerType.Silver;
    }
}

public class MySpec {
    private static bool HasMinNameLength(Customer c)
    {
        return c.Name.Length > 1;
    }

    public static bool IsSatisfied(Customer c)
    {
        return CustomerPredicates.IsSilver(c) && HasMinNameLength(c);
    }
}

public class Program
{
    public static void Main()
    {       
        var customers = new List<Customer>
        {
            new Customer { Name = "C1", Type = CustomerType.Silver},
            new Customer { Name = "C2", Type = CustomerType.Silver},
            new Customer { Name = "C3", Type = CustomerType.Gold }
        };

        /*
        Predicate<Customer> hasMinNameLength = c => c.Name.Length > 1;
        // Use closure to simulate accessing some other arcane specification
        Predicate<Customer> isSatisfied = c => CustomerPredicates.IsSilver(c) && hasMinNameLength(c);
        var result = customers.FindAll(isSatisfied);
        */
        var result = customers.FindAll(MySpec.IsSatisfied);

        Console.WriteLine(result.Count);
    }
}

seems like a much less cumbersome way to move forward.

[Khalid BOURZAYQ]

I completely agree with you on YAGNI.

Also i know that applying the Open-Closed Principle EVERYWHERE is wasteful, unnecessary, and can lead to complex, hard to understand code and fully agree with it.

But seriously I’m not a fun of duplicated code and the is a lot of solutions to avoid that !

The idea behind my article was to explain how we can implement the OCP principle inside a software in different ways. This is an article to simplify things to software developers in my point of view.

And in the conclusion a noticed that :

we shouldn't be afraid to do so, it's completely normal, but we should at least make these changes as discrete as possible.

I can’t see what was wrong ?

To conclude :
I think, we both agree that SOLID are principles and not rules!!

Thank you for these comments which have raised this thread and i hope that it will be conclusive debate.

[peerreynders]

I'm not a fan of duplicated code

DRY isn't about removing repetition or duplication: "every piece of knowledge must have a single, unambiguous, authoritative representation within a system". Sometimes code can look similar but is unrelated.

(POV: Development by Slogan with DRY: Part 2, The Tower of Coupling)

I can’t see what was wrong ?

The setup.

If the article would have started by stating that it was known up front that both ByType and BySubscription filtering would be needed and that other, yet to be determined kinds of filtering may still need to be added in the future, that would have laid out the background of a clear and necessary dimension of extensibility that OCP could apply to.

As it is, it was presented as if OCP somehow should make it obvious which dimension of extensibility is needed when only ByType filtering is required. Initially there are no actual dimensions of variability indicated. That only happened once the BySubscription requirement surfaced later.

This sets up the unrealistic expectation that IFilter<T> should be introduced at the very beginning (which is revisionist) when in fact the value of that abstraction only becomes clear much later.

In a realistic scenario you would start with FilterByType and end up at Apply - which means that the code isn't "closed to modification" as you are "opening it for extension".

[Khalid BOURZAYQ]

In my example i apply OCP just in time and this what should we do and i Refactor to Open-Closed.

Yagni only applies to capabilities built into the software to support a presumptive feature, it does not apply to effort to make the software easier to modify.

YAGNI

At first reading the open closed principle may seem to be nonsensical. Our languages and our designs do not usually allow new features to be written, compiled, and deployed separately from the rest of the system. We seldom find ourselves in a place where the current system is closed for modification, and yet can be extended with new features.

The Open Closed Principle - The Clean Code Blog by Robert C. Martin (Uncle Bob)

In OCP, the term module includes all discrete software elements, including methods, classes, subsystems, applications, and so forth. Also, the phrase “closed with respect to X” means that clients are not affected if X changes.

Protected Variation:The Importance of Being Closed

Indeed, most commonly we add new features by making a vast number of changes throughout the body of the existing code. We’ve known this was bad long before Martin Fowler wrote the book[2] that labeled it Shotgun Surgery but we still do it.

[2]Hall, 1988. [2] Refactoring, Martin Fowler, Addison Wesley, 1999

The Open Closed Principle - The Clean Code Blog by Robert C. Martin (Uncle Bob)

[David Kröll]

What would be the difference to an implementation with the C# LINQ type Predicate<T> except the strong typing of your filters?

I'm thinking of an implementation like this:

public class FilterServiceWithPredicate
{
    private readonly Predicate<Customer> _filter;

    private List<Customer> customers { get; set; } = new List<Customer>
    {
        new Customer { Name = "C1", Type = CustomerType.Silver, Subscribtion = SubscribtionType.Free},
        new Customer { Name = "C2", Type = CustomerType.Silver , Subscribtion = SubscribtionType.Basic},
        new Customer { Name = "C3", Type = CustomerType.Glod, Subscribtion = SubscribtionType.Basic}
    };

    public FilterServiceWithPredicate(Predicate<Customer> customerFilter)
    {
        _filter = customerFilter;
    }

    public List<Customer> FilterCustomers(Predicate<Customer> predicate)
    {
        return customers.Where(predicate.Invoke).ToList();
    }

    private Predicate<Customer> CustomersPredicate(CustomerType t)
    {
        return new Predicate<Customer>(c => c.Type == t);
    }

    private Predicate<Customer> CustomersPredicate(SubscribtionType s)
    {
        return new Predicate<Customer>(c => c.Subscribtion == s);
    }

    public List<Customer> GetSilverCustomers() => FilterCustomers(CustomersPredicate(CustomerType.Silver));

    public List<Customer> GetCustomersWithFreeSubscibtion() => FilterCustomers(CustomersPredicate(SubscribtionType.Free));
}

[Khalid BOURZAYQ]

My thoughts on Predicate Method:

• It can be used when you need a lot of control over queries..
• Good for small projects, where specifications may be overdoing it.

My final thought logic is that if you are working on a complex business application where business requirements are known up front but may change over time, then i would use Specification pattern.

The use of a design pattern It gives you highest modularity and flexibility and of course testability

[John Peters]

In C# the extension method is used as a solution to OCP. Over the years we've learned the principles of the Repository pattern which is, in essence to make all collections generic. This satisfies the ability to inject any type into a collection. Finally Language Integrated Query ability gives us precise filtering.

[Khalid BOURZAYQ]

The open/closed principle is a guideline for the overall design of classes and interfaces and how developers can build code that allows change over time. By ensuring that your code is open to extension but closed to modification, you disallow future changes to existing classes and assemblies, which forces programmers to create new classes that can plug into the extension points. That’s the idea ;)

[John Peters]

Yes, and my point is C# has a built-in construct that automatically creates extensions. It is very cool.