As a software engineer, I still remember my very first code review.
For those unfamiliar with code reviews, they’re basically peer review sessions of code written by a programmer, by other programmers. The key is it’s a PEER review of other coders … not a review by your manager, not a review by a QA person, business analyst or even coders on different dev teams.
It’s a review by team members, who you feel comfortable working with and trust, with similar levels of technical and business domain skills as you. The purpose of a code review isn’t to belittle the programmer who wrote the code. It’s main purpose is to look for ways to improve the way the code was written. And in the process, hopefully help the original programmer improve the way they write code in the future.
All that said, my first code review was STILL a nerve racking affair, to say the least. I was working for an PAAS (Platform as a service) company at the time and fairly still new to the company as a newhire employee.
I don’t remember the specifics of the code, but I clearly remember how nervous I was, my code displayed on a projector screen for all my fellow developer coworkers to see.
I’ve taken creative writing courses during my college years, and the review process is surprisingly similar in nature.
Everyone in the code review takes turns, round robin style, and discusses what confuses them about the code, what works well in the code, what doesn’t work well, suggestions about improving the code.
When all was said and done by each code review member of the team, the original programmer of the code could speak up and discuss their own thoughts.
When the flashlight is full on focused on your code by your peers, it can be a very scary experience. At least for myself, it was very hard to not take the review personally.
In a proper code review, it should ONLY be about ways to improve the code. But if you’re not careful, the code review can end up being about critiquing the PERSON, not the CODE. And that can be a very dangerous and demoralizing thing to do.
Unfortunately, I’ve been in code reviews that were more about critiquing the coder, not the code … these kind of reviews ended up being big ego sessions where the loudest developer would actually mock someone else’s code they would think was inferior. It’s truly a POISONOUS type of review that does more harm than good.
Fortunately for me, my first code review was a positive experience, overall. There was definitely lots of things that my peer reviewers brought up during the review that have still stuck with me today.
One particular critique hit on a key design principle, called SRP (Single Responsibility Pattern). My section of code under review, was doing WAY too many different things it shouldn’t have been doing.
For example, if one of my classes handled logging various events to a log file, it should have no business doing other things like connecting to a database, or printing documents out to an external printer.
Each class should be responsible for ONE thing and ONLY one thing. If a class is responsible for logging, it should ONLY contain code that deals with logging. If a class is responsible for printing, it should ONLY contain code dealing with printing.
Why? So that there are no unwanted ripple effects when you need to change something. And you only have to change the code in ONE location.
This single design principle has stuck with me to this day.
The underlying principle behind code reviews is that a second set of eyes (or better yet, even more) will have a better chance of catching flaws in code, and ideally, ways to IMPROVE the code.
The more developers participating in a code review, the more experience and expertise you draw upon. As a developer, I am confident there will always be ANOTHER developer with more experience and expertise than myself. Code reviews allow for the opportunity to benefit from that expertise and help you GROW as a developer. As you gain experience and your own expertise, you can repay the favor by passing it on your own expertise as a participant in another code review.
I’ve learned it’s important as the focus of a code review, to leave your ego and personal feelings at the door.
As a code reviewer, it’s important to keep the following things in mind
1. What goes around comes around
Remember you are not only dealing with the code, you are dealing with the PERSON who wrote the code. If you like to relish whaling on other people’s code and finding their flaws, just remember you’ll be on the hot seat, sooner or later and face the same thing by your peers. You’ll want to be as CONSTRUCTIVE as possible … don’t just point out the flaws, offer suggestions on how to IMPROVE the code.
2. Critique the code, not the style
Some developers like to critique the coding STYLE of a program, not the actual code.
Things like where to put the curly braces in languages like C, C++, C#, Java or Javascript
public void MyMethod { //open curly brace starts on same line as method declaration
}
instead of:
public void MyMethod
{ //open curly brace starts on the line after method declaration
}
There’s lot of other “styles” of code, which every developer prefers over other styles.
Critiquing this sort of thing seems a huge waste of time to me. As long as the style is consistent in a program, coding style should stay outside of code review critiques.
3. You pick and choose what suggestions to follow
You don’t have to follow EVERY suggestion and critique of a code review. It’s your code and you’re ultimately responsible for the way it’s written. It takes time and experience to pick and choose the RIGHT suggestions and critiques to apply to your code.
Conclusion
So I survived my code review. I think it should almost be a ground rule that you have to get your code reviewed FIRST before you start reviewing other people’s code.
That way you get to FEEL the heat of a review before you turn it on someone else.
Nobody appreciates a Simon Cowell, do they?
