Twitter LinkedIn Github

JetBrains

…of course, you’d need to have the song ABC from the Jackson Five in your head for that title to be remotely amusing.

Single Responsibility Principle is such a simple principle. It states that a class should only have one responsibility. One responsibility. Not two, not three, one. Such a concise and simple definition is hard to get wrong, right? Right? RIGHT??? Hmm…

clip_image001

A conversation between two friends

I’m looking at this Customer class, it does a bit too much no?

Huh? What do you mean? It’s a Customer Class. It deals with Customers. That’s all it does. Not Employees. Customers. I can fetch a customer. I can fetch a lot of customers. I can store a customer. I can calculate their age. I can check if they are VIP customers. Everything there is to do with customers. That’s all it does. Isn’t that what this Single Responsibility Principle states? To only have one responsibility?

Well, yes, but I think you’ve kind of missed the point on what this whole responsibility thing is .

How so? It just deals with Customers right? In fact, isn’t what I’ve just described pretty much the ActiveRecord Pattern?

Indeed it is.

So then WTF is the problem? I don’t get it.

Digging deeper…

Hmm, let’s look at this class. It reads/writes a customer(s) from/to the database. Therefore it has some code that has to do with reading from a database. It calculates age and verifies if a customer is a VIP. So it must have some business logic code in there.

What happens when you need to change the persistence logic?

Well I’ll change the code.

Which code?

The Customer class of course.

Right. What happens when you need to change the business logic of how a customer is considered a VIP?

I’ll change it again…..Sorry. What’s your point?

So again, you’ll touch that class. What happens when Customers need some validation? You’ll again have to touch the same class.

And?

What happens when you touch code? You usually end up breaking things. And it’s not necessarily that particular thing you touch. You break other things that at that point you had no clue were somehow related. The more complicated the code, the more chances you have of breaking something. Of course, if on top of everything,  you don’t even have regression tests…well I’m sure you’ve suffered the consequences.

I kind of think of myself a competent developer. I’m sure I can work with enough care to not break things.

Right…. Now while you go and convince yourself of that, let me explain another problem. You see, writing code is easy. Understanding it is hard. The more code in a class, the harder it will be to decipher. The less code, the easier it will be to comprehend. If a class does many things, it will most likely have what? More code. The less it does, you got it! Less code. Forget logic for a moment. It even has psychological impacts. Open a large class and a small one. Which one will depress you more?

I have no problem understanding my own code. 

Sure. You’re very smart and you’ve worked on this code base and you know it well. What about other team members? Hell. Forget others. Why don’t you look at code you wrote yourself eight months ago. Do you know what it does?

Well mostly yes. But I guess that’s kind of why comments are useful

Why do you need comments if your code is clear enough to understand? And you know why your code is hard to understand? Because you’re trying to solve too many problems at the same time. Think about it. You’re doing Customer listings, storing, reading and some business logic. It might be the case that for this particular example it’s not too hard to understand, but that’s because both of us have beaten Customer management to death (along with Authentication) so we’re experts in it. But imagine being thrown into some code that you have little knowledge of the domain. Is it easier to understand a class if it does one thing or five? 

So what are you proposing?

It’s actually very simple. Divide and Conquer. Remember that? Back from our College days? Divide up a problem into smaller problems and solve the smaller issues. Dealing with mini-problems is easier than dealing with mega ones.

When we take a class and divide it up into smaller classes, it will be easier to deal with it. And dealing can mean both understanding it and modifying it. But if your customer class is doing all that, it’s bound to be harder to understand and maintain. That’s why it’s important to get these responsibilities right.

I see. But where have I gone wrong then? I kind of thought I understood the Single Responsibility Principle. Going back to my Customer class, how do I define what the responsibility is if it’s not “dealing with customers”?

Getting a grip on responsibilities

The main problem in complying with SRP is defining the responsibilities. Where do we draw the line? A few things that have helped me along the way have been…

Change

Have you ever heard people say:

“A class should only have one reason to change”

when talking about Single Responsibility?

Yes, actually I have. 

Well think about it. If a class changes for more than one reason, it must be because it’s doing more than one thing. Going back to your customer class, think how many reasons there is for it to change. If we have to touch the same class for changing different things, the risk of breaking something is higher, which leads to higher costs in maintainability.

One way therefore of trying to figure out the responsibilities of a class is by asking the question of how many reasons are there for it to change.

Naming

Naming is a good way to discover the responsibilities of a class. What does your class do? Give it a name describing what it does. Does it use And or Or? Is it hard to explain what it does? Do you need to use suffixes like Manager, Processor or Admin because you can’t pinpoint the exact word describing what it does? Maybe it’s because it does two things that one word can’t describe. These are all clues that your class is doing more than one thing.

Cohesion

Take your class. Look at your methods. Do they have parameters or are they using instance fields? If they are using parameters, remove them. Make them instance fields. Do you end up with methods that only use one of the five instances? That most likely is a warning of the low cohesion that exists between that method and your class. Cohesion is a indication of how well related lines of code are, how well related methods are to a class. 

And you use ReSharper right. Next time ReSharper prompts you if you want to make a method static, it’s telling you it does not use instance fields. That’s where you need to decide if that method actually belongs in that class. Sometimes it might, many times it might not.

 

OK, that’s kind of making sense.

Wait though. Many look at Single Responsibility Principle as something that applies only to classes. Wrong. It applies to methods too. The more things your method does, the more lines of code. The more lines of code. The harder to understand. The harder to debug.

Use the same process to identify responsibilities in methods. How many reasons do I have to change the method? What do I name it? If I can’t name it on a single line, bad! If I can’t name it without using And/Or , bad!

Refactor your methods to smaller ones. Give each smaller method a good name. Make it descriptive. Don’t waste time trying to comment your methods and the 20 parameters it takes. If it’s well named and has one or two parameters at most, it should be self-describing. Remember, parameters are used to operate on values, not decide what path to take in the method. For that, you create two methods.

(Beer enters the scene…)

Hahahhaha…

What?

I remember back when I first started using non-locking source control, I was concerned about running into conflicts when merging. I was right. I did run into issues. I bitched at the SCM. But now that I think of it, the problem wasn’t the SCM. It was the way I was working. It was having too much code in the same class.

Yep, and that would lead to contention since many people would need to touch the same code.

Yep. I was fighting the wrong problem. Damn, it seems such a simple principle, but hard to get right then….

Yes. It’s about asking the right questions. Single Responsibility Principle is about making code maintainable and understandable. That’s all there is to it really.

Right. Well dude, I guess it’s time for me to Refactor out that Class.

Yep. Oh and do yourself a favor. If you’ve not read the book Clean Code by Robert C. Martin (@unclebobmartin), get it. It teaches you much of what we’ve been talking about.

Cool!