Article summary
You’re most likely familiar with the Single Responsibility Principle (SRP)-it’s the S in the SOLID acronym and a cornerstone of many folks’ object-oriented software design guidelines.
If you’re like me though, you’ve pretty much always heard SRP discussed in relation to modules or classes. I had started thinking in an SRP way when writing Clojure functions, but I hadn’t made the mental connection until I read Bob Martin’s recommendation in his Clean Code book.
A Brief Example
What does it mean to have a single responsibility for a function? As Uncle Bob describes it, branching is one thing, error handling is one thing, managing a loop is one thing. Let’s work through an example to illustrate.
Imagine we have the following function which takes a list of users. If the user list is empty, it prints an error and returns. If it’s not empty, it walks the user list, creating new data for valid users and skipping over invalid users.
(defn construct-win-percentage-data [users]
(if (empty? users)
(do
(println "Use a real logger here maybe")
:invalid-data)
(->> users
(map
(fn [user]
(when (:name user)
(try
{:name (:name user)
:win-percentage (/ (:wins user) (:games user))}
(catch Exception e
(println "Error processing user:" user)
nil)))))
(remove nil?))))
An example invocation might look like:
(construct-win-percentage-data
[
{:name "Frank" :wins 1 :games 3}
{:name "Francis" :wins 2 :games 4}
{:name "Charles" :wins 0 :games 0} ; Divide by zero!
{:nope :nah}
])
The function contains roughly 15 lines of code, which you might argue is pretty reasonable, but there are actually a whole lot of responsibilities being handled in there:
- The
users
collection passed is checked to make sure it’s valid. - There’s error handling if the collection is empty.
- The code maps over the collection if it’s non-empty.
- It makes sure each user is valid before trying to process it.
- It creates new data when a user is valid.
- It catches exceptions when it fails to transform the data.
- Empty entries are stripped out.
I’d argue that it’s also somewhat time-consuming to understand exactly what the code is doing. I mean, someone familiar with Clojure will read through it and quickly see what’s happening, but it takes “parsing.”
One last note is that the function is testable, but it’d be kind of a pain to test thoroughly. It would be difficult to look at tests, look at the code, and immediately know if all relevant paths had been covered. You’d probably also need to do more in terms of mocking or passing carefully constructed test parameters to exercise paths than you’d like.
An Alternate Implementation
Consider this implementation instead:
(defn construct-win-percentage-data [users]
(if (empty? users)
(handle-empty-users-param)
(construct-data-from-user-collection users)))
(defn- handle-empty-users-param []
(println "Use a real logger here - Invalid users")
:invalid-data)
(defn- construct-data-from-user-collection [users]
(->> users
(map construct-data-for-user)
remove-empty-items))
(defn- remove-empty-items [coll]
(remove nil? coll))
(defn- construct-data-for-user [user]
(when (valid-user? user)
(safely-build-win-percentage-data user)))
(defn- valid-user? [user]
(some? (:name user)))
(defn- safely-build-win-percentage-data [user]
(try
(build-win-percentage-data user)
(catch Exception e
(handle-build-win-percentage-data-error e))))
(defn- build-win-percentage-data [user]
{:name (:name user)
:win-percentage (/ (:wins user) (:games user))})
(defn- handle-build-win-percentage-data-error [e]
(println "Do some real logging to print the error: " (.getMessage e))
nil)
Now each function does a single thing. It’s either a branch, or it does error-handling, or it has logic for checking a condition, etc.
Is This Better?
“OMG!!!” you might be thinking. “There’s been an explosion of functions!” And perhaps you like the look of the original code better.
But here are a few reasons I’ve really been enjoying this latter style:
- These functions are super-easy to test, and it’s trivial to see whether they’re well-tested.
- It’s almost impossible to duplicate code when writing this way–duplicate code leaps out and begs to be removed.
- It becomes very easy to see when a function doesn’t belong in a namespace–
valid-user?
, for example, jumps out immediately here. - This style of coding forces you to name what each step of your code is doing. This can take some getting used to. It can feel slow and difficult at first, but patterns emerge, and I find it makes the code much more readable over time. I don’t need to spend the mental milliseconds parsing the algorithm.
I’m finding myself writing more and more of my Clojure code this way, and it’s my novice opinion that Haskell functions encourage this approach. I heartily encourage you to give it a shot and see if you like it.
I am doing the same thing, actually no matter what programming language you use.
Code becomes more readable easy testable.
Keep going like this!
Absolutely agree with you.