Single Responsibility Principle for Methods

In my last post, you learned what the Single Responsibility Principle is, why it is important, and its benefits. You’re now able to apply SRP at the class level.

Today, I’ll show you how to apply SRP at the method level.

By looking at some practical examples, you’ll learn how to spot potential code smells so that you can refactor your methods into shorter methods. You’ll write shorter methods with each doing just one thing. This’ll make it much easier to write unit tests because the inputs and outputs become obvious. There are also fewer edge cases per method.

Class level vs method level

The original SRP states that:

Each software module should have one and only one reason to change.

Last time, we treated software module as class. This time, we’ll treat software module as method.

So, the SRP can be restated as:

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

What is reason to change?

Let’s say you have a method M that does several things: A, B, C, and D. Things A and B implement feature X, while things C and D implement feature Y.

When feature X’s requirements change, you’ll need to modify method M. While doing so, there’s a fair chance you’ll also inadvertently alter feature Y.

Your tests for method M have to cover for all test cases for feature X, feature Y, and a full permutation of both features X and Y. This makes it difficult to write the tests in the first place. They are also more fragile and break when a change is made.

How do you feel when you update feature X and unintentionally change the behavior of Y? You lose confidence in your tests. You keep juggle around in method M, hoping it’ll work again. Eventually, it spirals into a massive method M.

And then no developer, including you who wrote it in the first place, wants to touch method M again.

That’s no way to programmer happiness.

So how can we do better? Extract things C and D into a new method N. Invoke N from M.

M still does two things A and B. But it’s ok, because A and B are both necessary to implement feature X. Similarly, things C and D in N implement feature Y.

Now, you could separate A, B, C, D into four methods. But that’s an overkill.

The key is to maintain the cohesion between things that change for the same reason, and decouple the things that change for different reasons.

As Uncle Bob pointed out in his post on the Single Responsibility Principle, on the topic of cohesion vs coupling:

Gather together the things that change for the same reasons. Separate those things that change for different reasons.

A massive viewDidLoad() method

I recently ran a workshop and showed this massive viewDidLoad() method to a group of iOS developers.

How many things does this viewDidLoad() method do?

In short, when the view finishes loading, it tries to show the 5 most recent posts of a user’s followers. If the user isn’t logged in, it shows a login button.

Let’s break it down into bullet points:

  • When the view finishes loading
  • 5 posts
  • Sort posts by most recent first
  • Fetch posts over the network
  • Get the currently logged in user
  • Get the user’s followers
  • Show a login button if logged out

This viewDidLoad() method massively violates the SRP.

If you want to see how I break down this massive viewDidLoad() method into shorter methods, you can subscribe below to find out the details of future workshops.

Get the Clean Swift Xcode Templates

Subscribe below to get my Xcode templates and learn how to apply the VIP cycle to your projects, extract business and presentation logic into interactor and presenter, navigate to different scenes using multiple storyboards, and write fast, maintainable tests with confidence to make changes.

I promise I'll never send you spam. You can unsubscribe at any time.

Spotting keywords in feature spec for multiple responsibilities

When you look at a feature requirement, it is often useful to keep an eye out for some keywords that can give you clues into some potential code smells.

It should XXX and/or XXX.

  • The fetchPosts() method should check the login status and fetch posts and parse JSON data into post objects.

It should XXX when XXX.

  • The fetchPosts() method should parse JSON data into post objects when it fetches posts.

If XXX, it should do XXX.

  • If the user is logged in, the fetchPosts() method should fetch posts. If it successfully fetches posts, it should parse JSON data into posts objects.

The client can specify a feature requirement in any of these three versions. If you pay attention to the keywords before you begin to write the code, it’ll help you see multiple responsibilities from the beginning so you don’t end up writing massive methods.

Potential code smells to watch out for refactoring opportunities

For existing code, you can also detect potential code smells by noticing specific language constructs and how they are used.

Let’s consider the following accelerate() method for a Car.

The following language constructs are very common, and aren’t necessarily bad by themselves. What you need to focus on is the context in which they are used.

&&, ||

This should be obvious. If you AND or OR multiple things together, you could be doing more than one unrelated things. If they aren’t related, break them down into separate methods.

Here, checking the engine state and if the battery has charge are unrelated. They are also too implementation dependent. (We’ll look at pumping fuel next.) It can be improved to:

Now, if the determination of engine state and calculation of battery charge has to change, you only need to change in one place, engineStarted() and batteryHasCharge().

Now, you’re glad you’ve refactored it to separate methods. There’s just one place to change and it’s well tested.

if

If a condition is true, then do something. This is probably the most used statement in any application.

The problem arises when the condition becomes yet another thing. If do-first-thing-that-returns-bool, then do-second-thing.

If your condition is more than just checking state, you may be doing too many things. If that’s the case, refactor the first thing into a separate method that returns a Bool. The new method can then be reused and invoked in other places.

Another problem occurs when you nest too many if-then-else. It usually means you are doing one too many things at each nesting. Refactor the nested if into its own method.

Pumping fuel from the fuel tank through the fuel pump into the engine compartment is not merely checking state. It is doing something essential. It’s already refactored into its own pumpFuel() method. But we can do even better by moving it out of the condition and into the body.

In Swift, you use if let for optional binding, this is simply syntactical, not code smell.

switch

Swift makes switch very powerful. It can match all kinds of things such as numbers, strings, intervals, and even multiple values in tuples. It can also perform pattern matching and value binding for optionals. Finally, you can use where to check for conditions.

To quote the Spider-Man,

With great power comes great responsibility.

You have to be careful not to mix too many things together in a switch statement. Prefer cleanness over coolness. Complex pattern matching is hard to read and understand.

If you are doing too many things in a matched case, refactor it to its own method with a descriptive name for reusability and testability.

The whole acceleration algorithm is embedded in the accelerate() method. These nitty gritty details of physics and geometry should be refactored to a lower level to be individually tested.

Even better, refactor the switch statement to its own method and give the method a meaningful name for readability and cleanness.

When you use switch for pattern matching, try to keep the cases simple and use where judiciously. The pattern matching algorithm should be refactored into its own method with a meaningful name such as findMakerAndModelFromVIN().

for, while, repeat

Loops can smell similarly to if and switch. You have a condition to determine if the loop should repeat. The condition is usually pretty simple and not a problem. But if you do too many things inside the loop, you may want to take a closer look.

In the while loop of the accelerate() method, we are doing way too many things. For every time step,

  • Get the surface and tire materials
  • Calculate the friction of the road on the car
  • Calculate the acceleration of the car
  • Calculate the distance to travel
  • Figure out the next position of the car
  • Move the car to the new position

Do all these physics and geometry need to coexist in the accelerate() method? Not really. So let’s refactor it.

The original accelerate() method becomes much simpler and shows only the steps at the high level. The physics and geometry lower level details are extracted. You can write unit tests to make sure all the complexity is fully tested and verified to work. If you need to make a change later, you have a test suite to rely upon.

guard

guard is great to ensure some requirements are met before the method continues. Just make sure you don’t specify too many requirements. If that’s the case, refactor these requirements into separate methods.

Alternatively, you can extract the whole guard statement into its own method that return a Bool. They can then be individually tested and reused elsewhere.

Checking if the car can move is likely a very useful assertion elsewhere, so it makes sense to refactor it out.

Again, this increases reusability and testability for the canMove() method.

a ? b : c

The ternary conditional operator:

is a shorthand for:

Since it’s a shorthand, it is supposed to be short. If you make it too complex, or even nest them, it kinds of defeat the purpose.

Some people love these tertiary operations because they are concise. But use with caution. Make sure your condition and branches are simple. If you have to && or || multiple conditions, execute more than one statement in a branch, or calling multiple methods, they are almost certainly too complex. They can greatly reduce readability and are hard to understand.

Number of lines of code

While this isn’t syntactical, but it’s still a factor to consider. When your method starts to grow and consists of many lines of code, it’s time to consider if you are doing too many things.

Method size may indicate you are specifying unrelated things to happen after one another.

On one hand, it’s fine if a task requires multiple lines of code to initialize, configure, execute, and return results. That’s just the verboseness of the language or class design.

On the other hand, if you find yourself having more than one algorithms in a single method, you may want to refactor them into multiple methods and invoke them from your original method.

Get the Clean Swift Xcode Templates

Subscribe below to get my Xcode templates and learn how to apply the VIP cycle to your projects, extract business and presentation logic into interactor and presenter, navigate to different scenes using multiple storyboards, and write fast, maintainable tests with confidence to make changes.

I promise I'll never send you spam. You can unsubscribe at any time.

One response

  1. Great post, Raymond!
    I feel you experience when I read it. Sometimes these ideas come into mind but lazy can kill it. So lazy is evil. Now I know that do small methods with SPR it a right way, and you need to force yourself to do that.
    Thanks.

Leave a Reply

Your email address will not be published. Required fields are marked *