SOLID principles in .NET revisited part 5: the Liskov Substitution Principle 2

Introduction

In the previous post we looked at letter ‘L’ in SOLID, i.e. the Liskov Substitution Principle. We saw how adhering to the LSP helps remove implementation-specific details from a client class that consumes those implementations. The client class will be able to consume the services of an abstraction without worrying about the actual implementation of the abstraction. The goal of LSP is that any implementation of an abstraction will work seamlessly within the consuming class.

We looked at the following cases that can indicate a violation of LSP:

• switch or if-else blocks checking for the value of an enumeration
• Code blocks that check the actual type of an abstraction to branch logic. This can be coupled with downcasting the abstraction to a concrete implementation type
• Blocks with magic strings that check for a value in some string parameter and branch logic accordingly

There’s at least one more indicator which we haven’t seen so far. I decided to devote a separate article to that case as it brings us closer to the next constituent of SOLID, i.e. the Interface Segregation Principle.

Throw new NotImplementedException()

Recall that we have the following AuthorizationService:

public class AuthorizationService
{
	public bool IsAuthorized(string username, string password)
	{
		return (username == "admin" && password == "passw0rd");
	}
}

Imagine that the requirements have changed. The implementation of the OnDemandAgentService.StartNewOnDemandMachine() method must be extended in a way that unauthorized users must be locked out of the user store. Furthermore, the company wants to store all users in an SQL Server DB.

By now we know that hiding concrete implementations behind abstractions can be beneficial so we’ll take that approach here:

public interface IAuthorizationService
{
	bool IsAuthorized(string username, string password);
	bool LockOut(string username);
}

We also come up with a glorious DB implementation of the interface:

public class DatabaseAuthorizationService : IAuthorizationService
{
	private List<string> users;

	public DatabaseAuthorizationService()
	{
		users = new List<string>();
	}

	public bool IsAuthorized(string username, string password)
	{
		return true;
	}

	public bool LockOut(string username)
	{
		return users.Remove(username);
	}
}

We add the usual property getter and setter to OnDemandAgentService:

private IAuthorizationService _authService;

public IAuthorizationService AuthorizationService
{
	get
	{
		if (_authService == null)
		{
			_authService = new DatabaseAuthorizationService();
		}
		return _authService;
	}
	set
	{
		if (value != null)
		{
			_authService = value;
		}
	}
}

…and we also extend the StartNewOnDemandMachine method to accommodate the new business requirement:

public OnDemandAgent StartNewOnDemandMachine()
{
	EmailService emailService = new EmailService();
	LoggingService.LogInfo("Starting on-demand agent startup logic");
	try
	{
		if (AuthorizationService.IsAuthorized(Username, Password))
		{
			LoggingService.LogInfo(string.Format("User {0} will attempt to start a new on-demand agent.", Username));
			OnDemandAgent agent = CloudProvider.StartServer();
			emailService.SendEmail(string.Format("User {0} has successfully started a machine with ip {1}.", Username, agent.Ip), "admin@mycompany.com", "email.mycompany.com");
			return agent;
		}
		else
		{
			bool lockedOut = _authService.LockOut(Username);
			LoggingService.LogWarning(string.Format("User {0} attempted to start a new on-demand agent. User locked out: {1}", Username, lockedOut));
			throw new UnauthorizedAccessException("Unauthorized access to StartNewOnDemandMachine method.");
		}
	}
	catch (Exception ex)
	{
		LoggingService.LogError("Exception in on-demand agent creation logic");
		throw ex;
	}
}

So this works well for the time being. However, we don’t want to forget about our previous authorization service. We want to be able to use that as well within OnDemandAgentService. However, it doesn’t have any user store so a lockout logic is not really applicable to it:

public class AuthorizationService : IAuthorizationService
{
	public bool IsAuthorized(string username, string password)
	{
		return (username == "admin" && password == "passw0rd");
	}

	public bool LockOut(string username)
	{
		throw new NotImplementedException();
	}
}

If we set this implementation through the AuthorizationService property setter of OnDemandAgentService then StartNewOnDemandMachine will stop working when calling…

bool lockedOut = _authService.LockOut(Username);

…since it throws a not implemented exception. We’re clearly violating LSP. The client object cannot simply consume the abstraction like it did before. We’re degrading the functionality as soon as we pass in the AuthorizationService concrete implementation for the IAuthorizationService dependency.

An attempt to remedy the situation is to implement the interface in a way that the exception is not thrown:

public bool LockOut(string username)
{
	return true; //or false
}

However, that would raise subtle bugs. It returns true every time and then we may be wondering why the same user is still allowed to perform other operations in the system. If, on the other hand, it returns false all the time then we might go look for the error why the user couldn’t be locked out. Hence we introduced some bogus behaviour in our implementation just to avoid throwing an exception somewhere in the system. We’re effectively forcing AuthorizationService and DatabaseAuthorizationService to be on the same level of the “IS-A” relationship. DatabaseAuthorizationService definitely satisfies the “IS-A” relationship, i.e. a DatabaseAuthorizationService is an IAuthorizationService because it can unambiguously fulfil the IAuthorizationService contract. AuthorizationService can only partially do that so we can say the AuthorizationService “IS-NOT-REALLY-A(N)” IAuthorizationService.

What can we do then? One obvious solution is of course to introduce some kind of backing store for the users in AuthorizationService so that the LockOut method makes sense for it.

Another solution is to break up the IAuthorizationService interface according to the next constituent of SOLID, which is the Interface Segregation Principle. More on that in the next post.

View the list of posts on Architecture and Patterns here.