SOLID - The S is for Single responsibility

March 23, 2012 at 8:26 AM - by Freek Lijten - 9 comments

Tags: Test driven development, SOLID

What?

The single responsibility principle isn't all that hard to explain. It states that an object should do one thing, and one thing only. Or as Robert C. Martin (see props below) puts it:

There should never be more than one reason for a class to change.

He follows through with:

Why is it important to separate these two responsibilities into separate classes?
Because each responsibility is an axis of change

These two quotes are all there is to the single responsibility principle. A responsibility is a reason to change, and a class should only have one of those. Now all of this may sound abstract and since the objective of this series is to avoid just that we'll just dive into the why now.

Why?

Now for the fun part, code :)

Lets take a look at a concept called active record. In short these are smart models, they can create, read, update and delete themselves. In other words they represent a row in a database. This is a concept that is widely used inside and outside the PHP community. Code using active records tend to look like this.

$Bike = new Bike();
$Bike->wheels = 3; //this is a very special bike
$id = $Bike->save(); //create

$Bike = new Bike($id); //read
$Bike->wheels = 2; 
$Bike->save(); //update
$Bike->delete(); //delete

The save function will probably look somewhat like this:

public function save()
{
    //magically get DB-object
    if ( ! empty($this->id)) {
        //update DB
    } else {
        //create in DB
        $this->id = mysql_insert_id();
    }
    return $this->id;
}

Apart from that, the Bike class also does another thing, it is able to compare two bikes and determine if they are equal (the implementation is simple, you'll get the idea):

public function equalTo(Bike $Bike)
{
    return $this->wheels == $Bike->wheels;
}

The setup above, a model of some object that stores itself and has one (or more) convenient methods isn't all to uncommon. In fact it seems very convenient and doesn't look all that bad. It will be working more than fine. But it isn't what you want...

Reasons to change (multiple responsibilities)

You probably will have noticed the multiple responsibilities this class has, and thus the multiple reasons for changing the code:

1. A change of datastore
2. A change in the way bikes are compared
3. A change in the properties a bike consists of (the only way you'd want for a model)

A clear violation of the Single responsiblity pattern as this class obviously does 3 things, it models a bikes, it stores a bike, and it compares bikes. But why should you care?

Problems

This approach results in several problems, I'll list them below.

Datastore lock-in

The model is now tied to the datastore. Easily exchanging MySQL with something else is not possible without changing this (and every other) active record. As we'll see later, this will give us problems while testing as well. It would of course be possible to inject a datastore object, but you'd still have to write the queries. If you'd also inject the way the object is saved to the datastore (queries) why bother with an active record at all? Might as well create some sort of repository for it.

Encourage other Single responsibities breaks

The datastore is now available inside the Bike class. Not only does this encourage developers to query the datastore for other reasons than CRUD (Create, Read, Update, Delete) inside the Bike class, but it also causes another problem.

Having the datastore available inside the Bike class implicitly encourages the direct use of the datastore in other places as well. You end up with the datastore being used all over the place, making the datastore lock-in even worse.

Too much information

This Bike will soon be an ObfuscatedBike. Already code for CRUD and comparing will be in the way of the definition of the bike and this will only grow. When you want to add a function to your code that merges two bikes for instance, what will be the obvious place?

In the current setup almost everyone would choose to add it to Bike. And you will for the next functionality too. And the one after that. These kind of classes tend to be hundreds of lines long, while it only started with a simple save function. I've read and wrote these classes and they're a pain, period.

Unit test problems

Let's assume we want to write a unit test for the compare function. Using PHPUnit, you would probably do something like this

require_once '/path/to/Bike.php';

class BikeTest extends PHPUnit_Framework_TestCase
{
    public function testBikesAreEqual()
    {
        $Bike1 = new Bike();
        $Bike1->wheels = 2;
        $Bike2 = new Bike();
        $Bike2->wheels = 2;

        $this->assertTrue($Bike1->equalTo($Bike2));
    }

    //not equal test be here
}

Seems ok enough right? But since the Bike class depends on the datastore this would probably result in some sort of fatal error.

1. The database class may be autoloaded by your framework, but the framework is not loaded when PHPUnit executes.
2. If the database class is required at the top of the Bike class it probably can't be constructed (missing configuration it would have when inside your framework for instance).
3. I actually ran into the first to problems on occasion, but perhaps there are even more you can think of.

So now we must start fixing code while all we wanted to do was test the function that compares Bikes. A function that doesn't even need a datastore!

Now let's assume we would inject the datastore in the Bike class by passing a datastore to the constructor (or in every function that uses the database), then we would have to mock a datastore object to test the equalTo functions, or we'd have to write a hell of a lot of code each time to pass a datastore object to each function of the Bike using it.

I think by now you will start to see the problems with our Bike class, let's come up with a solution.

A better approach

We already know Bike has three responsibilities, so why don't we model our code to represent that:

class Bike
{
    public $wheels;

    public function __construct($wheels)
    {
        $this->wheels = $wheels;
    }
}

class BikeComparator
{
    public function bikesAreEqual(Bike $Bike1, Bike $Bike2)
    {
        return $Bike1->wheels == $Bike2->wheels;
    }
}

class BikeRepository
{
    public function save(Bike $Bike)
    {
        //do whatever
    }
}

So what have we reached here:

1. A Bike is just a Bike, even with a type of steering wheel, amount of gears, brand name and price added anyone will be able to see what a Bike consists of at a glance. 
2. The BikeComparator can be happily tested, it has no other Dependency than the Bike class itself which is perfectly valid, it is comparing Bikes after all. I can understand you might think this is silly, but keep in mind this is a simple example. Comparing multiple properties with different rules  will easily result in a lot of code.
3. The BikeRepository does the saving for us. It's only responsibility is storing and retrieving Bikes.

You might think that BikeComparator still has two reasons to change, as does the Repository. Add a wheel size for instance and you'd have to update both classes. In case of the BikeComparator this isn't necessarily true. The comparator only needs to change if the new property matters while making a comparison. If you want to compare between types (trikes, normal bikes and a skelter of some sort) and don't care about wheel sizes, the BikeComparator needn't change at all. It is safe to say that there is still only one reason for change.

The repository is a slightly different case. It needs to change if the bike gains a property or if the datastore needs to change (MySQL -> NoSQL or something similar). You can solve this in multiple ways, but it is a little outside of the scope of this post. What you could do is introduce a Repository class that defines the properties and types that need to be saved and then load it with different classes that provide the actual implementation of a save action in MySQL, NoSQL, whateverSQL, etc. Also take a look at ORM systems like Doctrine for this kind of problems.

Conclusion

I hope by showing a very small (but perfectly imaginable) example how annoying even the smallest violations of the single responsibility pattern can be. As the person who came up with the term once said (freely quoted):

Of all the SOLID principles the single responsibility principle might be most easy to understand, but it is the hardest to get right.

I think I'll end this article with that encouragement :)

Props

Robert C. Martin is the father of the term SOLID. I've read several of his books and articles and he is a very skilled writer:

• Robert C. Martin has a lot more for you to read here: http://www.butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod
• You could also buy one of his books: http://www.amazon.com/Robert-C.-Martin/e/B000APG87E