Single Responsibility Principle
The Single Responsibility Principle states that:
"every module or class should have responsibility over a single part of the functionality provided by the software"
This is fairly confusing though. Each module or class has responsibility for a single part of functionality of the software. Let's break this down a little bit. The functionality of software could be to display
Robert C Martin says it this way,
"A class should have only one reason to change."
Which is succinct. But complicated. A class should have only one reason to change. What exactly is a reason, and what is a change to a class. There are many ways to change a class. We won't dwell on these questions right this second.
How does Rails handle the single responsibility principle? How can we as developers uphold the single responsibility principle with the code that we write on top of the familiar Rails API?
Rails models can be tough to extend. One of the tenets of writing good RoR is to keep controllers "skinny" and people often move that excess code back into the model. Models tend to get complicated, and they get complicated quickly. As you extend them to do different things, we have to test each method, as side affects pile up and we end up with unmanageable models. Which are arguably worse than unmanageable controllers.
If you have been in the Rails community for almost any period of time I am sure you are familiar with the classic "Blog in 15 minutes Rails app". That allows you to post a new blog post and write comments about that blog post. It is simple and easy, traditionally, we have three models in this app. A Post, a Comment, and often a User or an Author.
The separation seems reasonable, in that there are two major things we need to do here and something needs to do those things.
The Post is the blog post, it includes:
- an author
- some information about when it was created
- the text about the blog post
The Comment is just as simple it includes:
- a reference to the post you are commenting on
- some information about the person that posted the comment
- the text of the comment
The Author/User that includes:
- identifying information
- magical rails associations that let you author comments and posts
The question here lies in what you expect the "Responsibility" here to be for each of these models.
Is it the Rails models responsibility to know where it is stored? What data is associated with itself. What to do you have a missing reference? What happens when each of the individual parts of a Comment or a Post are missing? How those models interact with other models.
Or you could take a bigger view, and say that the "single" responsibility here is that the model handles everything associated with the data that is the model.
We clearly know that it is not the responsibility to handle viewing itself, or to handle serving itself up at the correct end point.
Mr. Martin would say there should only be one reason for a Post or a Comment to change. What does this change entail? A change to the underlying data?
Rails is pretty good at this. A quick example could entail, the reason being that we updated, created, read, or deleted an instance of this model:
new_post = Post.new(user_id: 1, body: some_giant_wall_text)
# new_post is now a brand *new* instance of the Post class.
new_post.save
# new_post is now persisted to the database
new_post.update(body: some_other_giant_wall_of_text)
# new_post is now persisted to the database again!
What just happened? The same instance of the class was persisted twice? Did we just break the SRP in three lines of Rails code?!
We did in fact mutate the instance of the class. A quick sanity check does show that:
new_post.body == some_other_giant_wall_of_text
The instance of the class changed. The reason behind this change was that we persisted the data to the database.
Why would rails do this? It sorta feels like the model was doing two things here. Saving and updating. But let me rephrase what it was doing. The Rails model was handling the CRUD operations to the database. That is all. We created the model, but passing in a hash of some attributes, which is succintly saved to the database, we then updated those attributes and it also succinctly saved these attributes to the database.
The responsibility of the Rails model is to handle object in the database. The model allows developers to quickly handle CRUD operations against a database.
I think its safe to say that the reason for changing is that the Rails model was persisting something to the database. An attribute of the model changed, therefore the instance of the class can change and still fit the SRP.
The point is, that in many cases Rails Models are consistent with the SRP and you don't have to be scared that you're Rails app is not SOLID outta the box.
Our Rails Models inherit from ActiveRecord::Base for a reason. ActiveRecord handles the reading, creating, updating, and deleting of objects in a database. Nothing else.
But.
Rails models are hard. They get complicated. We all know this. We have all broken a model in a Rail app with unexpected consequences.
Often times business rules want our models to do other things, interact with other objects. When we want to allow our Post to suddenly infer who the author of the Post is. Or we want to only allow certain Users to see and interact with certain Comments. Or a Post suddenly needs to be a part of a posting board. Or you need to add Tags to a Post.
Rails does allow you to do this safely. It is quite trivial to add a new column to the database and create associations in the database safely.
This is seen on our Comments and Posts relationship.
post = Post.find_by(author: 'me') # an instance of a post from our Database
post.comments # responds with a collection of Comments that match the Post ID
post.user # responds with an instance of the author
post.tags # responds with and error since we have no `Tag` table
Once again most of this is an extension of letting us persist information to our database. We still have yet to break the SRP. Furthermore, we were able to extend our model in many ways and not break SRP.
We get pretty far doing this. Very far.
Now what happens when you come back with a requirement like, each Author should have a summary of the Posts they have made. Easy enough you say.
author = User.find_by(name: my_name) # find an author
author.posts # find all the posts
🎉 I have a collection of posts for that Author. Easy peasy.
Now I don't want the entire post! That is a lot of text for a summary. I only want the first 200 characters of each posts. Its a summary!
We end up reaching for a method on Post since that is the most convenient way to handle the data manipulation that must happen to display certain posts.
We end with a method on the Post model like:
class Post < ActiveRecord::Base
...
def body_summary
body[0..200]
end
...
end
So to get a Posts summary we can then call:
author = User.find_by(name: 'my_name') # find an author
author.posts(&:body_summary) # grab all the post summaries
But wait! What is the goal of ActiveRecord and our Rails model!? To Read, Update, Create, and Delete information from a database! Not to transform the data that we retrieve from the database.
Yes, now we have broken SRP in our Rails model. It was as simple as adding a single method. We have now modified the instance of the object, for a reason besides reading, updating, creating, and deleting values from the database. The database will never see nor care about the body_summary.
We are mutating an object that exists for a single reason, CRUD in a database, so that we can use it in a very different way, to display that information in a particular way in the view. This is a very, very common pattern to do in Rails. It is bad. :slaps hand away from writing crappy methods:
ActiveRecord models are for creating, reading, updating and deleting information from a database.
The body_summary itself is not the worst method in the world. My favorite Sandi Metz quote seemingly supports the existence of this code. In fact. I would encourage this effort in certain situations.
“The code is not perfect, but in some ways it achieves a higher
standard: it is good enough."
In this very simple example I would stop here and not really worry about it. Its not harming anything. Sure you could get into the N+1 query getting really heavy and taking a long time to generate the body_summary but chances are that is not a realistic possibility.
Besides the point. The code does break the single responsibility rule that we have defined so far. What are some Rails like ways that we can clean this code up?
Let's look at some other solutions so that our Post doesn't break the single responsibility rule. Starting from simple implementations to more complex solutions.
A simple and slightly better option here is to add a body_summary column to the Post table. Which would allow us to use a callback to add a body_summary each time we updated the Post.
class Post < ActiveRecord::Base
...
after_save :build_body_summary
def build_body_summary
update({body_summary: body[0..100]})
end
...
end
This is pretty simple. Personally I like the simplicity here and it keeps all the information in a single table and for use on the Post now when we want the body summaries we can access them like any other attribute on the Post object. But we are also assuming alot about the body here. What happens when build_body_summary fails? We need two validation cycles just to save the Post once.
Thinking about the single responsibility principle with respect to this solution the Post does save itself and all its own data. But does a Post need to be responsible for saving itself!? Does the Post class even have enough information to save itself each time? It must recall itself and then change by transforming itself into something that is close to, but not quite itself. Did it change with the express purpose of creating, updating, deleting, or reading? No, it changed to generate a body_summary that it then persists to a database.
Another option is to define a BodySummary class that is backed by a database table.
class BodySummary < ActiveRecord::Base
belongs_to :post
belongs_to :author
end
and extend the Post class with a callback. This would be a very Rails only way to keep the SRP in check. I would advocate the after_save callback because it would eliminate the need to overwrite the initialize method or overwrite other methods on the Post class.
class Post < ActiveRecord::Base
...
has_one :body_summary
after_save :persist_body_summary
def perist_body_summary
BodySummary.new(post_id: id, author_id: author.id, summary: body[0..100])
end
...
end
This would allow us to use standard rails forms to build out a collection of summaries for each of the Posts. But there is now a pretty major dependency of BodySummary smack dab in the middle of the Post class.
This is almost worse than having a body_summary method on the Post class. Heading back to the single responsibility one could in fact argue that it is outside the responsibility of the Post class to persist anything in the database beyond itself and therefore we are breaking SRP here too. I don't totally agree that BodySummary itself constitutes an explicit break in the SRP if the intent of the Post class is to create, read, update, and delete data for the Post. It just happens to be in a different table than the Post. What are your thoughts on this?
A lot of people have talked about service objects lately. In fact there has been some recent kerfuffles by some heavy hitting Rails enthusiasts about the use of Service Objects.
The discussion on Service Objects and their overall necessity in Rails is a topic for an entire other post with many people on all sides advocating for both options.
I do enjoy a good service object and we could easily write a PORO that took a collection of Posts and returns the body summary.
class PostBodySummary
attr_reader :posts
def initialize(posts:)
@posts = posts
end
def perform
summaries = []
posts.each do |post|
summaries << post.body[0..200]
end
summaries
end
end
Calling PostBodySummary.new(posts: user.posts).perform is very trivial and does the trick. One fall back here is we have a pretty limited understanding as to what is exactly going on with this model. If we don't crack open the PostBodySummary its pretty unapparent what we are doing. One way we can alleviate some of this pain is be renaming the perform method to something like generate_summaries. But ymmv in this simple case.
This is easily reusable and extensible. We could define a different method and it would return shorter summaries. Easily enough we could write:
class PostBodySummary
...
def shorter_summaries
posts.each do |post|
post.body[0..100]
end
end
...
end
This is cleaner and follows our pattern above of explicitly returning what you call.
We should check in on the single responsibility principle here for a moment. What does the PostBodySummary class do? It takes an ActiveRecord Collection and returns the summaries of the bodies of the Posts. When does the PostBodySummary change? It only changes when we want a different ActiveRecord collection of Posts. This seems reasonable. One Author's set of Posts is different than a different set Author's Posts and each instance of the service object would reflect this change. This solution is probably the solution that I think follows the single responsibility principle the most. PostBodySummary changes when the collection of Posts changes. The one reason to change, is that the collection of Posts changes and this object reflects this.
Overall thinking about the single responsibility principle is tough. Especially when we are ready to extend the API that Rails exposes for us. Some people would say that I missed the point by talking too much about Rails models and that the single responsibility principle isn't so much about instances of objects and the way we use them as it is about the actual code in the class. I digress.
Rails models are tough to extend when they get complicated, part of the reasoning is that they continually break the single responsibility rule even in simple cases. Generally the goal of a Rails model is to handle the data and the database. I think this is complicated enough that, that is all it needs to do.
There are many options that let us encapsulate the complications of Rails models outside of the actual model. Some of my favorite are presented including utilizing callbacks to set an attribute on itself, using a Service object to completely remove the code, adding a new ActiveRecord object itself, and some options that fit in between.
Let me know what you think about the single responsibility principle!
-------- Some more commentary on the SRP by the man himself:
Uncle Bob's clarification to what he actually meant when he said:
"A class should have only one reason to change."
https://8thlight.com/blog/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html
-------- Some more info on Service Objects and the articles I mentioned earlier:
Aaron Lasseigne's piece on using service objects in response to Avdi's post:
https://aaronlasseigne.com/2017/11/08/why-arent-we-using-more-service-objects-already/
Avdi Grimm's piece on service objects and procedures:
https://avdi.codes/service-objects/
One thing to note, Aaron is the author of a very popular Service Object gem called ActiveInteraction. So that could possibly jade his overall opinion.
Top comments (1)
Thanks for the article, and here is my 2 cents
In this scenario having a service object for getting a data is not friendly.
It would be hard to remember to look into the service objects, when we want to understand the domain model context
The main question is why do we need this short_summary in the first place? Is this only for the view part? Or we need it to be a strong part of the domain model context? Answering that question will lead us to few possible solutions:
I think service objects serve a bit different purpose, but I'm opened for the discussion :)