However, a tool won’t be useful until we use it according to its design and intended purpose.
What are the main reasons behind doing a code review? In my opinion they include sharing knowledge (learning and teaching) and increasing readability and understandability of the written code.
Today I want to show you the negative impact on the quality of a review that explaining the changes introduced in the code may have.
Let me explain it...
Do you know those eager developers who write tons of comments in their code review? They’re adding a comment to almost each line explaining what and why was implemented. Sometimes they also include a portion of information that explains why it’s done in this and not that way. Do you know them?I believe the intention that drives them is good. They want to minimize the effort needed to understand the code. They want the best for the reviewer. They want to avoid wasting time. Great, isn’t it?
Well, their intentions are good, but the result of such activity is not. Why?
As I wrote in the first paragraph, one of the values of the code review is increasing its readability. How can we verify whether the code is descriptive enough or not? By reading it. If we can understand what was written that tells us the code is self-explanatory. If not, then it can be a sign that some renames may be needed.
And now tell me how we can verify whether we would understand the code by reading it if everything is already explained and does not require any additional effort from our side?
Asking for an explanation
In the project I’m working in there was once a suggestion that if you cannot understand the code you should ask its author for explanation. Well, I would go for something entirely different.If you ask for an explanation, you decrease your chances for increasing readability of the code. How you can consider whether code is understandable or not if you already understand what and why? Of course, there will be some obvious things you still may notice, however, the most problematic parts of the functionality was already explained. It is hard to challenge readability if the code was explained. There is just no need for additional descriptiveness.
Ask the expert
It would be an unrealistic idea if I advised you not to ask at all. Sometimes we have patchy knowledge. Does not it matter whether those gaps are about technical aspects or domain logic.When such situation happens I would suggest that you don’t go directly to the author of the code. Firstly, you should look for the answer on your own. If, after carrying out the research, you still don’t have enough information, it is always a good idea to ask for help someone more experienced than you. You will gain knowledge and such learning process won’t have any negative impact on verifying the descriptiveness of the written code.
What about the context?
Sometimes you also may like to ask a question to find out what’s the main reason behind the change. The code in the review usually is only the part of the functionality author wants to add to the code base. If we don’t know what is the main goal it may be hard to do the review properly. To justify the change.That’s why I found it very useful to clarify the purpose and talk with someone who knows more about it. It does not have to be an author, but it may be. However, it is important to remember that we should ask not about code in the review, not about this particular part of the whole, but about the main change that stands behind the small one.
Think about values, but...
You have to remember what impact explanation of the code in the review may have on the review’s quality. Yet, you also have to remember that sometimes you won’t avoid it. Sometimes you just need to know more, sometimes you need some input from the author of the code. And it is still fine.I’m just asking you to delay those questions a little bit. Try to do a review without their explanation. Ask other people for help. And only if you are pretty sure there’s still some piece of information that is missing, ask the author for the explanation.
No comments:
Post a Comment