Intro
This post is not about the many benefits of practicing code reviews within a development organization. I found that many articles deal with these benefits but tend to neglect the way code reviews should be conducted.
To be honest, I’m still learning how to give and receive code reviews myself. It is an on going process for me, but I managed to gather some rules along the way to form a basic code of conduct that I try (yes, try. I don’t always succeed) to follow.
In this post I would like to share these with you —
Below there are 7 guidelines, 6 of which end with a simple question that sums it up. I call it The “The question I need to ask myself before commenting on a code review”, or The “pre-commenting question”.
I should stop trying to make the reviewed code look as if I wrote it
Nobody can write code the same way I do , nor should they. We all should follow the same standards we agreed upon, but it will never be the same.
One of my biggest mistakes was trying to make the code I’m reviewing be as if it was written by me. You know what I mean, right? I’m talking about commenting over whether to use ternary logical statement (or not) in a certain spot, or assigning an expression to a variable instead of using it in its raw form later on.
If I have a strong reasoning why something should be a certain way then yeah, I should comment about it, but that’s not always the case. The pre-commenting question should be:
Does the comment I’m about to write has a strong reasoning behind it or is it a matter of personal preferences?
Avoid being Holier Than Thou
I had to accept, as a rule of thumb, the fact that there is always something to do in order to make the code better, for as we know — The best code is the code not written. Writing code is balancing between efficiency, quality and maintainability, and each of us developers have a slightly different set of priority weights for these aspects.
The problem starts to rise when we review code. I’ve noticed that when I review code my own priorities tends to change. Suddenly coding mistakes, which I let fly under the radar when I wrote some code, seriously disturb me now — when someone else have made them.
True story: I once copy/pasted some code and got a code-review comment on the content from the developer who authored the source I pasted from.
In this case there are 2 available options — either I comment about it and fix every place I did the same or suggest modifying the code very nicely (and perhaps admitting my weakness as well). Preaching what you don’t practice really sucks. The pre-commenting question should be:
Do I deserve such a comment myself?
Be focused on the reviewed code’s objective
Code has context.
While we review code modification we try to look at the modified bits but you got to have the context to see the broader picture. The thing is that sometimes I look at the contextual code and see some wrongs. I see that the code is written with bugs or in bad need of a refactor, and perhaps it does, but that’s when I need to remind myself to focus on the objective of the current modifications and review the relevant code modifications. If I think that there is a need to refactor or change something outside that scope, it is better to take it offline the code review and prevent the noise. The pre-commenting question should be:
Does my comment relate to the modified code and its objectives?
Be suggestive, not decisive
Even though we’re dealing with software development, which is supposed to be a form of scientific implementation, in many cases choosing the right direction to go is not obvious and there are grey areas. Sometimes the direction we choose very much depends on the context the code runs in. Once I understood that there is more than a single way on doing stuff my comments begun to be more suggestive, e.g. “Please consider replacing…” or “It might be a good idea to…” instead of “Replace this” or “Do that”. Receiving such a comment or agreeing with it is also much easier on one’s ego.
If I do find something that makes me go “Why the hell did he do that?!” I stop to realize that there is a slight chance that it was done with a good reason behind it, and so instead of “Why the hell did you do that?!” I might ask for the reasons, e.g. “I think the implementation is wrong due to… , what was the reason behind it?”. The pre-commenting question should be:
Might there be a reason the author chose this implementation?
Use references when needed
Sometimes I sense that a certain comment might be a little controversial , or maybe realize the comment relates to some newly discovered facts not commonly known — In these cases I find it useful to add a link to an article or Stack Overflow thread which might shed some light on what I’m talking about. I’m not saying your comments should drown in links, but if you know of an elaborated explanation that might help the author get you, why not include it? It also shows that you took the time and effort to be clear.
The pre-commenting question should be:
Would it help the author understand if I add an additional reference?
Give positive feedback when deserved
Everybody likes to get a good feedback on their work, but “Wow! that’s an amazing change!” over changing var into const is not what I would call “giving a positive feedback” on a code review.
I found that when the author makes a modification that was not required for the code to work (like refactoring, cleaning code, re-naming and the list goes on) positive feedback spontaneously comes out. Forcing it, just for the sake of having it, is something I really try to avoid. The pre-commenting question should be:
Does code change really deserve a positive feedback?
If there’s nothing to say — just say it
Sometimes the modified code leaves me speechless, in a good way. In this case I try to practice leaving a single comment saying something like “Looks good to me, I have nothing to comment about :)”. Why? cause not leaving a single comment might be taken as if I didn’t review the code or didn’t pay attention to it. More over, leaving a positive comment like this (when deserved) is always a nice thing to receive. No pre-commenting question here ;)
Conclusion
Code reviews are another interface among developers and as such I believe it should be treated with the same importance over how it is conducted.
I’m pretty sure you have your own guidelines or other points of view over what I’ve written — be sure to share them on the comments :)
Cheers
Top comments (0)