SOLID – Single Responsibility

Def. A class should have only a single responsibility

(Robert C. Martin)

“S” in SOLID acronym stands for Single Responsibility Principle.
The definition seems quite obvious, intuitive and simple. But it’s not.
Let’s clarify it. In order to proceed, I will introduce 2 new terms:

Business Responsibility is business requirement, functionality that describe public class’ interface. It usually comes from client’s requirement, functional spec. or is implicitly formed as necessity by other functionalities.

Implementation Responsibility  is implementation requirement, class’ behavior that is forged by implementation design. It is needed as one of steps for full-filling the business responsibility. It still may be internally public, for re-use purpose.

Having these terms in mind, let’s analyze typical mistakes that developers usually make:

• A module/class has too many business responsibilities piled up together for no reason
  Making a “universal” tool (AKA ‘God Class’) in practice turns out to be very unforgiving mistake over time. Multiple responsibilities coupled together interconnect, while each of them grows in complexity. From design point of view, that blurs the intended purpose. From maintenance point of view, it’s a classic spaghetti.
• A module/class couples very similar, but unrelated business responsibilities together
  Two or more functionalities may be similar in variety of things: by end results or goals, or perhaps few steps in algorithm may seem to be common. But it is quite often a deception. Using ambiguous and too abstract terms to describe them is usually the cause. Explicit re-definition and separation of such functionalities is often the best solution.  Otherwise, just parameterized versions of same logic may do just fine.
• A module/class has a single business responsibility, but many implementation responsibilities
  It is also know as temporal coupling – Class is simply too complicated for usage and exposes too many of internal properties or behavior. Typical code smell is when you have sets of 2~3 methods always called one after another in the same order in multiple places. Encapsulation or different design patterns may be the cure, depending on the situation.

1. Coupling too many business responsibilities

Let’s analyze why too many business responsibilities put together is wrong.
One possible code example taken from .NET is MembershipProvider class.
When modified, it is used to provide common authentication & authorization scenarios:
login, logout, password retrieval, CRUD operations for users, etc.
All these functionalities are put together in one single class that you need to extend (dummy sample):

public class TestMembershipProvider : MembershipProvider
{
	public override string ApplicationName { get; set; }
	public override bool EnablePasswordReset { get; }
	public override bool EnablePasswordRetrieval { get; }
	public override int MaxInvalidPasswordAttempts { get; }
	public override int MinRequiredNonAlphanumericCharacters { get; }
	public override int MinRequiredPasswordLength { get; }
	public override int PasswordAttemptWindow { get; }
	public override MembershipPasswordFormat PasswordFormat { get; }
	public override string PasswordStrengthRegularExpression { get; }
	public override bool RequiresQuestionAndAnswer { get; }
	public override bool RequiresUniqueEmail { get; }
	public override bool ChangePassword(//...) { //... }
	public override bool ChangePasswordQuestionAndAnswer(//...) { //... }
	public override MembershipUser CreateUser(//...) { //... }
	public override bool DeleteUser(//...) { //... }
	public override MembershipUserCollection FindUsersByEmail(//...) { //... }
	public override MembershipUserCollection FindUsersByName(//...) { //... }
	public override MembershipUserCollection GetAllUsers(//...) { //... }
	public override int GetNumberOfUsersOnline() { //... }
	public override string GetPassword(//...) { //... }
	public override MembershipUser GetUser(//...) { //... }
	public override MembershipUser GetUser(//...) { //... }
	public override string GetUserNameByEmail(//...) { return null; }
	public override string ResetPassword(//...) { //... }
	public override bool UnlockUser(//...) { //... }
	public override void UpdateUser(//...) { //... }
	public override bool ValidateUser(//...) { //... }
}

Now, this class is always painful to implement. What if it was designed a bit differently?
Please, take a look at the following sample:

public class TestMembershipProvider : MembershipProvider
{
	public override string ApplicationName { get { //... } }
	public override bool RequiresQuestionAndAnswer { get { //... } }
	public override bool RequiresUniqueEmail { get { //... } }

	public override PasswordManagement PasswordManager { get { //... } }
	public override UserQueryManagement UserQueryManager { get { //... } }
	public override UsersUpdateManagement UserUpdateManager { get { //... } }
	
}

public abstract class PasswordManagement
{
	public virtual bool EnablePasswordReset { get { //... } }
	public virtual bool EnablePasswordRetrieval { get { //... } }
	public virtual int MaxInvalidPasswordAttempts { get { //... } }
	public virtual int MinRequiredNonAlphanumericCharacters { get { //... } }
	public virtual int MinRequiredPasswordLength { get { //... } }
	public virtual int PasswordAttemptWindow { get { //... } }
	public virtual MembershipPasswordFormat PasswordFormat { get { //... } }
	public virtual string PasswordStrengthRegularExpression { get { //... } }
	
	public virtual string GetPassword(//...) { //... }
	public virtual bool ChangePassword(//...) { //... }
	public virtual bool ChangePasswordQuestionAndAnswer(//...) { //... }
	public virtual string ResetPassword(//...) { //... }
}

public abstract class UsersQueryManagement
{
	public virtual MembershipUserCollection FindUsersByEmail(//...) { //... }
	public virtual MembershipUserCollection FindUsersByName(//...) { //... }
	public virtual MembershipUserCollection GetAllUsers(//...) { //... }
	public virtual int GetNumberOfUsersOnline() { //... }	
	public virtual MembershipUser GetUser(//...) { //... }
	public virtual MembershipUser GetUser(//...) { //... }
	public virtual string GetUserNameByEmail(//...) { return null; }
}

public abstract class UsersUpdateManagement
{
	public virtual MembershipUser CreateUser(//...) { //... }
	public virtual bool DeleteUser(//...) { //... }
	public virtual bool UnlockUser(//...) { //... }
	public virtual void UpdateUser(//...) { //... }
	public virtual bool ValidateUser(//...) { //... }
}

Now, let’s analyze this code a bit. Code is split into blocks each of which is responsible for one single thing:

• main class (facade)
• password management
• user queries
• user updates

A membership provider that gives default and optional instance for each block is now much better solution. The code is more readable, easier to use and separation of concerns helps the maintainability of the code. Important thing to notice here is that we can let these separate classes grow each in its own way. It’s perfectly fine, since each of them they will always single responsibility, if we are just a bit careful.

2. Coupling similar, but unrelated business responsibilities

This mistake is usually made with poor separation in domain design having ambiguous domain entities and terminology. Other reasons may be striking similarities between code snippets, their implementation and control flow. I will give simple example with poor domain design. Let’s say we are building an E-Commerce solution where we have Users. They can sign up and login. That way they become Clients. By buying the first item, they become Customers. We also have a feature that first registered user becomes an Administrator. Since we decided that all users are saved in the same DB table called USERS, we design the domain where Users are entities while Client, Customer and Administrator are roles – just features of single existing entity. We may end up having a contract like this:

public interface IUserService
{
    User GetUserById(int id);
    void SignUp(OAuth auth, string authId);
    void Login(OAuth auth, string authId);
    bool IsAdmin(int userId);
    void AddToCart(int itemId);
    void InitiatePayment(int cartId);
}

but over time, the interface will quickly grow like this:

public interface IUserService
{
    User GetUserById(int id);
    void SignUp(OAuth auth, string authId);
    void Login(OAuth auth, string authId);
    void SetClientDetails(User user);
    void UpdateClientSubscription(string email);
    bool IsClient(int userId);
    bool IsCustomer(int userId);
    bool IsAdimn(int userId);
    void AddToCart(int itemId);
    Cart RestoreCart(int userId);
    void InitiatePayment(int cartId);
    void GetCustomerInvoice(int purchaseId);
    Products[] DisplayOffers(int userId);
    Cart[] DisplayHistory(int userId);
}

We quickly realize that the number of interface responsibilities grows rapidly. Why is that? Simply because we assumed that all functionalities related to anyone who is a user belong to users sub-domain. If we think a bit outside the box, we will quickly realize that life of administrator, client and customer is essentially very different and activities of one have almost nothing similar to activities of another. Being a user in a different role may be considered as being completely different entity. If we try to re-design this into further sub-domains that are clearly separated and mutually distinctive by entity names and attached behavior, we will come up more or less with the following code:

public class User { }

public class Client : User { }

public class Admin : User { }

public interface IUserService
{
    User GetUserById(int id);
    void SignUp(OAuth auth, string authId);
    void Login(OAuth auth, string authId);
    Role GetUserRole(); //admin, customer, client
}

public interface IClientService
{
    void SetClientDetails(Client user);
    void UpdateClientSubscription(string email);
}

public interface IPaymentService
{
    void AddToCart(int itemId);
    Cart RestoreCart(int userId);
    void InitiatePayment(int cartId);
    void GetCustomerInvoice(int purchaseId);
}

public interface IContinuityService
{
    Products[] DisplayOffers(int userId);
    Cart[] DisplayHistory(int userId);
}

This looks much better now, doesn’t it? All interfaces are now dealing with completely separated sets of functionalities and entities. The only thing in common is User base entity, which is only there to represent real life situation as accurate as possible – only common stuff like names, emails etc. are kept in base class while others are in separate entities. This also reduces confusing parts of the code like IsAdmin() or IsCustomer() methods which would introduce again coupling logic of different modules under one roof – ‘is this user the first one to register’ or ‘has this user at least one invoce’ etc. Although this example is similar to previous one, it still had starting criteria for decoupling and splitting functionalities, but it wasn’t enough – initial solution would slowly converge towards ‘God Class’ with blurred design idea.

3. Single business responsibility, but many implementation responsibilities

This mistake is caused by creating variations of functionality outside of its own code, usually by exposing and using class’ properties and behavior that could have been encapsulated without additional damage. There are other possible reasons, but the end result is the same: class functionality was used in simple, one-liner, self-explanatory calls, but after changes, the usage is preceded or followed by another calls of methods from the same class (either as pre/post actions calls or if statements that branch the logic). Let’s give an example by getting back to previous example of IUserService – the version with single User class and no other entities (all client, customer and admin properties are in Users entity). Now, let’s analyze the following code for creating or updating users:

public class User
{
    //this class contains properties for all roles
    //there are no separate entities for separate roles
}

public class UserService
{
    public void UpdateClientDetails(User user) { ...}
    public void UpdateCustomerDetails(User user) { ...}
    public void UpdateAdminDetails(User user) { ...}
}

//usage in public surface API component:

public class UserController
{
    [Route("users")]
    public void UpdateUserDetails(User user)
    {
        if (_userService.IsClient(user))
        {
            _userService.UpdateClientDetails(user);
        }
        else if (_userService.IsCustomer(user))
        {
            _userService.UpdateCustomerDetails(user);
        }
        else /*user is admin*/
        {
            _userService.UpdateAdminDetails(user);
        }
    }
}

The decision behind this design is simple: details of each role are pretty complicated to validate and update so we decide to keep them in separate service methods. Good enough! – one would say… It is good enough if we don’t expect another role to be implemented (soon) or we don’t duplicate similar branching logic in another context (of users creation based on different roles assigned). But, let’s do both things. Let’s say we need to introduce users creation and another level of customers called Contributors – customers granted with permission to write significant product reviews. Those are yet another sort of users falling into a new role. The code becomes like this:

public class User
{
    //this class contains properties properties for all roles
    //there are no separate entities for separate roles
}

public class UserService
{
    public void UpdateClientDetails(User user) { ...}
    public void UpdateCustomerDetails(User user) { ...}
    public void UpdateContributorDetails(User user) { ...}
    public void UpdateAdminDetails(User user) { ...}

    public void CreateClient(User user) { ...}
    public void CreateCustomer(User user) { ...}
    public void CreateContributor(User user) { ...}
    public void CreateAdmin(User user) { ...}
}

//usage in public surface API component:

public class UserController
{
    [Route("user"), HttpMethod("PUT")]
    public void UpdateUserDetails(User user)
    {
        if (_userService.IsClient(user))
        {
            _userService.UpdateClientDetails(user);
        }
        else if (_userService.IsCustomer(user))
        {
            _userService.UpdateCustomerDetails(user);
        }
        else if (_userService.IsContributor(user))
        {
            _userService.UpdateContributorDetails(user);
        }
        else /*user is admin*/
        {
            _userService.UpdateAdminDetails(user);
        }
    }

    [Route("user"), HttpMethod("POST")]
    public void CreateUser(User user)
    {
        if (_userService.IsClient(user))
        {
            _userService.CreateClient(user);
        }
        else if (_userService.IsCustomer(user))
        {
            _userService.CreateCustomer(user);
        }
        else if (_userService.IsContributor(user))
        {
            _userService.CreateContributor(user);
        }
        else /*user is admin*/
        {
            _userService.CreateAdmin(user);
        }
    }
}

Now, this code is still not as bad as it could be. The code is defying open/closed principle by being so prone to change in the future. Also, the branching logic is code duplication. Still, all those mistakes are kept in the same file so are easy to control. On the other hand, if CreateUserDetails() and UpdateUserDetails() methods were in different classes, and even worse, developed by different people/teams, we would instantly introduce a mine field of future bugs! e.g. new roles are being added and we forgot to extend both places of code etc. The problems started from the moment we decided to split ‘users being maintained different basing on role’ functionality in the separate UserService methods – which is totally fine, but it introduced another problem – each time we need to use _userService, we need to repeat branching logic of checking for user type. This is called temporal coupling and class is now complicated to use in a proper way. Slightly better solution (and probably good enough) would be to encapsulate branching logic into private method in UserService and expose only  UpdateUserDetails() and CreateUser() methods:

public class User
{
    //this class contains properties properties for all roles
    //there are no separate entities for separate roles
}

public class UserService
{
    public void UpdateUserDetails(User user)
    {
        PresetUser(user);
        //Update logic
    }

    public void CreateUser(User user)
    {
        PresetUser(user);
        //Creation logic
    }

    //Setting only specific properties basing on role
    private void PresetUser(User user)
    {
        if (IsClient(user))
        {
            //...
        }
        else if (IsCustomer(user))
        {
            //...
        }
        else if (IsContributor(user))
        {
            //...
        }
        else /*user is admin*/
        {
            //...
        }
    }
}

//usage in public surface API component:

public class UserController
{
    [Route("users"), HttpMethod("PUT")]
    public void UpdateUserDetails(User user)
    {
        _userService.UpdateUserDetails(user);
    }

    [Route("users"), HttpMethod("POST")]
    public void CreateUserDetails(User user)
    {
        _userService.CreateUser(user);
    }
}

If we wish to expose presetting logic to the outer world and eradicate branching logic, we can push further with design decision. For example, builder pattern could help us out to decouple presetting logic into separate builders and expose them individually. For that, maybe the best solution is to move IsClient(), IsAdmin() etc. into User model class itself (contributing against ‘anemic model’ anti-pattern). Take a look at the following solution variation – it is flexible enough even for breaking into role based sub-modules that we could use on the individual basis and special contexts:

//builders implementation

public abstract class UserBuilder
{
    public abstract void Preset();
}
public class ClientBuilder : UserBuilder
{
    private readonly User _user;

    public ClientBuilder(User user)
    {
        _user = user;
    }

    public override void Preset()
    {
        if (user.IsClient())
        {
            //...
        }
    }
}

public class CustomerBuilder : UserBuilder
{
    private readonly User _user;

    public CustomerBuilder(User user)
    {
        _user = user;
    }

    public void Preset()
    {
        if (user.IsCustomer())
        {
            //...
        }
    }
}

//here we would have similar builders for contributor and admin

public class UserService
{
    public void UpdateUserDetails(User user)
    {
        PresetUser();
        //Update logic
    }

    public void CreateUser(User user)
    {
        PresetUser();
        //Creation logic
    }

    //Setting only specific properties basing on role
    private void PresetUser(User user)
    {
        new ClientBuilder(user).Preset();
        new CustomeBuilder(user).Preset();
        new ContributorBuilder(user).Preset();
        new AdminBuilder(user).Preset();
    }
}

//usage in public surface API component:

public class UserController
{
    [Route("users"), HttpMethod("PUT")]
    public void UpdateUserDetails(User user)
    {
        _userService.UpdateUserDetails(user);
    }

    [Route("users"), HttpMethod("POST")]
    public void CreateUserDetails(User user)
    {
        _userService.CreateUser(user);
    }
}

Summary

Single responsibility is simple rule that manifests in different ways in code. The best way to learn about it is to be aware of common mistakes, to be able to recognize and categorize them. It’s a design practice of building modules/classes basing on only one business responsibility specific to the domain, not implementation. Implementation details (AKA implementation responsibility) should be encapsulated and kept private inside, but if that is not possible or not desired, it should strictly have a single purpose to help full-filling business responsibility. If implementation is shared, it should be contained and re-used in sub-module defined for that business responsibility. The common mistakes are chaotic pile of functionalities, coupling similar, but not related business responsibilities and making modules/classes too complex for usage by exposing internal state and behavior. Solutions are domain model re-design, encapsulation and finally design patters.