Code Review and Single Responsibility Principle
The author connects the single responsibility principle with code review to simplify commits.
Join the DZone community and get the full member experience.Join For Free
According to the Single Responsibility Principle, each unit in our code should have only one reason to change.
Code Review, on the other hand, is a technique that helps us improve the quality of our code and increase its readability.
I believe that you know both the principle and the technique.
The question is, why do I juxtapose them in one article?
Well, if SRP is about one reason to change code, then I think that code in the review should be organized around one change. The author should organize the commit of the code which is sent to the review in a way that will reflect only one change.
Code Review and the Effort
Code review is really useful and important, but requires some amount of effort from the reviewer. A developer that reads the code will have to understand what needs to be implemented. They need to know whether expectations are met. They need to have enough understanding to verify the quality of the code and its readability. All of it requires both time and attention. During the review, if you want to do it right you have to stay focused.
That’s why we should make code review as easy as possible and decrease the effort that must be put into this activity.
Code Review and Only One Change
Many changes mean that you are jumping from one place to another and you are trying to figure out which change is the reason for which modification of the code. It makes reading the code harder, because you need to keep in mind many contexts and reasons. It makes it harder to find bugs in this situation. More things that are merged can go unnoticed.
When the code is put to review it has various reasons for modifications, and we need to carry out two additional activities: switch contexts and group changes into subsets organized around each change. And we also have to remember that each change can have an impact on another.
So the amount of effort and time that a reviewer needs to invest into a code review is:
Number of changes * effort needed for one change + switching the context + understanding implication
I've heard more than once that “it is better/easier/faster to do one code review instead of many, because you will do a review only once”. As you can see, though, putting many things into a code review is not a simple multiplication of effort needed for one review.
Unfortunately there is additional complexity in this equation.
Many Changes in One Review
The additional complexity of the review is not the only thing that stands against putting many changes into one commit. Many changes in one commit means less readable history, so when you want to find something, it would be harder because some changes will be merged with other changes. What would be the name of your commit in that case? It will need to either focus on the most important change or it will be something meaningless, because you will try to use one sentence to explain many things that were modified.
This doesn’t help when you have to find a root cause of some issue or you want to understand some part of the functionality. It is hard because of the same reasons that make the review of such code hard — complexity and presence of many contexts.
One Change at a Time
What do you think about this idea?
Let’s organize our commits around a single change.
Let’s make things simple and easier for us and all of those who will read the code.
Published at DZone with permission of Sebastian Malaca, DZone MVB. See the original article here.
Opinions expressed by DZone contributors are their own.