SOLID In C# – Open/Closed Principle

This article is part of a series on the SOLID design principles. You can start here or jump around using the links below!

S – Single Responsibility
O – Open/Closed Principle
L – Liskov Substitution Principle
I – Interface Segregation Principle
D – Dependency Inversion

What Is The Open/Closed Principle?

Like our post on Single Responsibility, let’s start off with the Wikipedia definition of the Open/Closed Principle :

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

Probably not that helpful to be honest. It’s sort of an open ended statement that could be taken to mean different things for different developers.

In looking at resources for this article, I actually found many write ups talking about how being able to substitute one implementation of an interface for another was an example of being Open/Closed, and indeed Wikipedia touches on this :

During the 1990s, the open/closed principle became popularly redefined to refer to the use of abstracted interfaces, where the implementations can be changed and multiple implementations could be created and polymorphically substituted for each other.

I’m not the right person to start redefining decades of research into software design, but I personally don’t like this definition as I feel like it very closely resembles the Liskov Principle (Which we will talk about in our next article in this series). Instead I’ll describe it more loosely as :

Can I extend this code and add more functionality without modifying the original code in any way.

I’m going to take this explanation and run with it.

Open/Closed Principle In Practice

Let’s take our code example that we finished with from the Single Responsibility article. It ended up looking like this :

public class RegisterService
{
	private readonly IUserRepository _userRepository;
	private readonly IEmailService _emailService;

	public RegisterService(IUserRepository userRepository, IEmailService emailService)
	{
		_userRepository = userRepository;
		_emailService = emailService;
	}

	public void RegisterUser(string username)
	{
		if (username == "admin")
			throw new InvalidOperationException();

		_userRepository.Insert(new UserModel());

		_emailService.SendEmail(new Email())
	}
}

Looks OK. But what happens if we want to also check other usernames and reject the registration if they fail? We might end up with a statement like so :

if (username == "admin" || username == "administrator")
	throw new InvalidOperationException();

Is this just continue on forever? Each time we are actually modifying the logic of our app and we are probably going to have to regression test that past use cases are still valid every time we edit this method.

So is there a way to refactor this class so that we are able to add functionality (Add blacklisted usernames for example), but not have to modify the logic every time?

How about this! We create an interface that provides a list of Blacklisted Usernames :

public interface IRegistrationServiceConfiguration
{
    public List<string> BlacklistedUsernames { get; }
}

Then we inject this into our RegisterService class and modify our register method to simply check against this list.

public class RegisterService
{
    private readonly IUserRepository _userRepository;
    private readonly IEmailService _emailService;
    private readonly IRegistrationServiceConfiguration _registrationServiceConfiguration;

    public RegisterService(IUserRepository userRepository, IEmailService emailService, IRegistrationServiceConfiguration registrationServiceConfiguration)
    {
        _userRepository = userRepository;
        _emailService = emailService;
        _registrationServiceConfiguration = registrationServiceConfiguration;
    }

    public void RegisterUser(string username)
    {
        if (_registrationServiceConfiguration.BlacklistedUsernames.Contains(username))
            throw new InvalidOperationException();

        _userRepository.Insert(new UserModel());

        _emailService.SendEmail(new Email())
    }
}

Better right? Not only have we removed having hardcoded strings in our code (which is obviously beneficial), but now when we extend that list out of usernames we don’t want to allow, our code actually doesn’t change at all.

This is the Open/Closed Principle.

Extending Our Example

The thing is, we’ve taken a list of hardcoded strings and made them be more configuration driven. I mean yeah, we are demonstrating the principle correctly, but we are probably also just demonstrating that we aren’t a junior developer. Let’s try and take our example that one step further to make it a bit more “real world”.

What if instead of just checking against a blacklisted set of usernames, we have a set of “pre-registration checks”. These could grow over time and we don’t want to have a huge if/else statement littered through our method. For example changing our code to look like this would be a bad idea :

public void RegisterUser(string username)
{
    if (_registrationServiceConfiguration.BlacklistedUsernames.Contains(username))
        throw new InvalidOperationException();

    if (_userRepository.CheckUserExists(username))
        throw new InvalidOperationException("User already exists");

    _userRepository.Insert(new UserModel());

    _emailService.SendEmail(new Email())
}

How many statements are we going to add like this? And each time we are subtly changing the logic for this method that requires even more regression testing.

How about this instead, let’s create an interface for registration pre checks and implement it like so :

public interface IRegistrationPreCheck
{
    void Check(string username);
}

public class RegistrationPreCheckBlacklistedUsernames : IRegistrationPreCheck
{
    private readonly IRegistrationServiceConfiguration _registrationServiceConfiguration;

    public RegistrationPreCheckBlacklistedUsernames(IRegistrationServiceConfiguration registrationServiceConfiguration)
    {
        _registrationServiceConfiguration = registrationServiceConfiguration;
    }

    public void Check(string username)
    {
        if (_registrationServiceConfiguration.BlacklistedUsernames.Contains(username))
            throw new InvalidOperationException();
    }
}

Now we can modify our RegisterService class to instead take a list of pre checks and simply check all of them in our RegisterUser method :

public class RegisterService
{
    private readonly IUserRepository _userRepository;
    private readonly IEmailService _emailService;
    private readonly IEnumerable<IRegistrationPreCheck> _registrationPreChecks;

    public RegisterService(IUserRepository userRepository, IEmailService emailService, IEnumerable<IRegistrationPreCheck> registrationPreChecks)
    {
        _userRepository = userRepository;
        _emailService = emailService;
        _registrationPreChecks = registrationPreChecks;
    }

    public void RegisterUser(string username)
    {
        _registrationPreChecks.ToList().ForEach(x => x.Check(username));

        _userRepository.Insert(new UserModel());

        _emailService.SendEmail(new Email())
    }
}

Now if we add another pre-check, instead of changing the logic of our RegisterUser method, we just add it to the list of pre-checks passed into the constructor and there is no modification necessary. This is probably a better example of the Open/Closed Principle that demonstrates the ability to add functionality to a class that doesn’t require changes to the existing logic.

What’s Next?

Up next is the L in SOLID. That is, The Liskov Principle.