How to Be a Better Code Reviewer
How to Be a Better Code Reviewer
A couple of weeks ago, I gave you a few hints on how to be a better code reviewee. This time, we’ll look at the other side of code reviews - being a code reviewER.
Join the DZone community and get the full member experience.Join For Free
Don’t Block People
Code reviews, especially when held using online tools, can feel blocking for people as they might not want to start a new task when their previous one (or 7 previous ones as happened to me once) is still in review. It’s also desirable that the reviewEE has enough time to apply changes before the Sprint ends. Therefore, get down to do the code review in a timely manner.
Take Your Time
This point may seem contrary to the first, but in reality, it’s not. Once you find the time and sit down to do the code review, don’t hurry. How can you evaluate to the solution to make sure it is correct without its careful analysis, or if the code is readable if you don’t read most of it? This might not apply if there are other forces or constraints in place. In such cases, use common sense to balance between speed and quality of the review.
Whenever you find something wrong with the code, try to be as specific as possible. Try to articulate what exact change you suggest. If necessary and possible, you can even provide short code samples. There’s a high chance that the person who submitted the code believes it’s good enough. It won’t help if you write comments like Wtf? or Correct it, please (I've heard about such cases). I know that sometimes you see the code and it just doesn’t “feel right” and you don’t know why exactly. In such cases, try talking to a rubber duck or the code reviewEE about the ways things could go wrong with the solution.
Read the Tests
It’s so, so obvious and yet people fail at this (including myself, sometimes). We should care for the test code the same way we care about the production code. There are no clean, robust, and fast test suites that can be created without proper care being applied to develop them. Another thing is, you want to make sure that all test cases are covered. You can’t do that without actually reading the tests. And if you find some test case is missing, there’s a high chance that you just found a potential production bug.
Don’t Make It Personal
However hard they try not to be, people WILL be personal about the code they write. They will treat it as their lovely little kid and react defensively when the critique is too harsh. Therefore, try to make the code review feel like final you're polishing of the code, instead of trying to prove the other person's stupid. Avoid any kind of personal comments. Make it all about the code. When the solution is so bad that it requires a complete overhaul, try to make them feel like you simply have an alternative idea to their good one, not an alternative idea to their awful one.
“It’s not how I would have done it” is not an argument. Unless the change you’re suggesting is obvious, you should explain why you think the code should be changed and what would be the consequences of going either way. You don't have any special rights just because you’re a reviewER. If you want the code to be changed, you need to think how to “sell” the idea to the other person. Sometimes you might not feel good enough to explain everything by yourself. In these cases, try to provide some resources that may justify your opinion (just not 800-page books please!).
Don’t Make Sure It’s Perfect, Make Sure It’s Good Enough
As the code gets cleaner, the time passes and you keep thinking about the reviewed solutions, you might keep getting new ideas about how it could be improved. However, there needs to be a stopping point at which the code is accepted and all other changes will be applied in subsequent stories. The business wants to make money and so the project has to move forward. Therefore, work on improving the code until it’s good enough to make money for a long time, not to print it and hang it on the wall.
Prefer Discussion Over Online Comments
I know I’ve written that in the previous post about reviews, but it’s important and cannot be stressed enough. If you’re using an online tool for code reviews even though the team is co-located, try to limit the online comments to stuff that you both agree on. Once you recognize a disagreement, it’s better to take things offline. The kinds of problems that take hours and tons of comments can usually be resolved in a few minutes of productive talk. And you can drink some tea at the same time!
Let the Reviewee Know When You’re Done
Some online tools (like Stash) lack a way to let the reviewEE know that you’re finished with the review. In such cases, it’s good to employ some informal way of letting the other person know that you’re done, e.g. writing I’m done! or just saying it out loud. It’s a little thing which might sound stupid to offline review practitioners, but I find it really helpful.
Published at DZone with permission of Grzegorz Ziemoński , DZone MVB. See the original article here.
Opinions expressed by DZone contributors are their own.