I do a lot of code reviews as part of my work. If you’re a coder in an organization with more than one developer, chances are you do, too. If you don’t, you should start. You can learn a lot from code reviews, and I don’t just mean learning to be a better coder. You also learn about people you work with. And when your code gets reviewed, you can learn about yourself.
The thing that amazes me about code reviews is the weird perception some coders have about them, and what they think they are.
Someone I worked with (who didn’t stay around long) used to submit the most awful code reviews. He held the top three worst code reviews in the history of the company, in terms of length of time they took to complete, the number of comments, and the number of commits to fix errors. The worst had 305 comments across 30 files, 60 commits, and took around three weeks to finally get merged. It was not a complex piece of work, and it wasn’t even his first code review.
To say it was frustrating is an understatement. Many of the things done were clearly not tested, because they would have broken immediately. Each code review repeated the same mistakes. Fixes were implemented and pushed that introduced further bugs, because those fixes weren’t tested, either. It was obvious that no thought had gone into his work. Worse, it created friction in the team, because each review took so much effort and time, that it impacted other work.
Amidst a myriad of excuses (“PHP Storm didn’t commit my files”, “requirements are conflicting”, “it worked on my machine”), he once remarked to me, “That’s what code reviews are for.”
It’s illustrative of exactly what code reviews are not. They are not for me to test your work for you. They are not for me to waste my time telling you to follow PSR-2 coding style. They are not for me to tell you that PHP Storm lights up like a Christmas tree when I open your file.
Code reviews are not there to allow you to be lazy and have me fix your mistakes.
A code review is like a piece of work you’ve just submitted for an interview. Would you submit something that makes the interviewer think you’re incompetent or lazy, or that he’s wasting his time? Then don’t do it to the person reviewing your work. They’re busy, too. They’ve got their own work to complete. They want to help you complete your work in the best way possible, not to do your work for you.
At a previous job, I had another colleague who once said he loved giving code reviews. The reason boiled down to the fact that he enjoyed trying to demonstrate how superior he was. It was no surprise that the same individual hated having his own code reviewed and would argue against every criticism, and viewed it as an attack on himself.
If you find yourself feeling defensive, code reviews can help teach you how to take constructive criticism. They force you to think about why you’ve done what you’ve done. When someone asks you why you’ve done it this way, and not that way, you should have a good reason. If you don’t, it may suggest you’re not thinking about what you’re doing, which may help you next time.
And on the flip side, if you are reviewing people, and they always get defensive, perhaps you’re being too harsh in what you’re saying, or you can word it differently. Code reviews should be there to help, not torture or judge. This is not the Spanish Inquisition. Check your egos at the door. Comments made should not be to berate anyone. They’re not a judgment of skill, so don’t see it as an attack, unless of course you’re guilty of such glaring errors that it demonstrates you’re not thinking at all about what you’re doing. Even then, a good code reviewer would be constructive in what they have to say and, hopefully, you’ll pay attention and not do it again.
A code review, at its core, is a collaboration. A good reviewer reads your code, and thinks of exceptional cases that you may have missed, or suggests a way refactoring what you’ve done that makes it better. A good reviewer will understand the requirements of what you’re supposed to deliver, and make sure nothing obscure has been missed. Perhaps there’s a library already being used somewhere you’re not aware of that could be used. Maybe your code can be greatly simplified.
Another colleague once remarked that code reviews were slowing him down, they took too long, and impacted his ability to deliver. Obviously, the question arises as to whether that’s the fault of the reviewer, or the fault of his code, but putting that aside, it’s true: code reviews take time. But they provide an overall net benefit. They prevent obvious bugs from hitting different environments. They improve the overall code quality, and can help keep technical debt down.
Perhaps most importantly, code reviews enable us to learn from each other. We learn how to communicate effectively, and how to work as a team. All parties involved can learn how to do things a better way, or learn about the domain they’re working in, which can sometimes be old, and complex.
If you approach code reviews as a learning exercise, over time, you’ll stop repeating the same mistakes, and generally improve your code. If you’re open to the concept of code reviews, you’ll open communication with your team members, and break down the work silos we can sometimes find ourselves operating in.