The first thing to know about code reviews is if you think they’re easy, you’re probably terrible at them. But don’t feel bad. Code reviews are incredibly hard to do correctly, but I have met a few people in my career who are exceptional at them. I’ll share what makes them rock stars a bit later.
What is a code review?
Before we get to any of that, let’s quickly cover what a code review is and its true purpose. According to GitLab, A code review is a peer review of code that helps developers ensure or improve the code quality. Ideally, these reviewers will be familiar with the code base in general and maybe even with the specific modules that are being worked on. It’s a chance to get another set of eyes on the problem. It’s the whole idea that two heads (or a room full of them) are better than one.
Code reviews are often done in a formal meeting environment. The developers responsible for a certain section of the code or a particular product are invited. Reviews can last anywhere from a few minutes to several days, depending on what’s being reviewed. However, you generally want to keep the amount of code short enough that everyone can mentally survive the meeting. That means it’s better to review code more often, rather than trying to review vast amounts of code in one sitting.
There are also “over the shoulder” reviews. This is where another developer will stop by your office to review code that you’ve written. This is most often done before you check the code in and is less formal than a meeting style review. Developers use over the shoulder reviews to do a quick pass with another set of eyeballs when shorter code is being submitted. They’re also often used as a quick sanity check when an urgent fix must go in quickly. You don’t have time for the more formal review, but it’s still better to make sure you’re not crazy before you push that new code into the wild.
What are code reviews for?
The purpose of a code review is to make the overall software code base better. Now “better” can be a pretty subjective word, so we need a little more definition around that. What makes good software? First, software that works. It’s also important that other people can read the software, so more than just one developer can work on it, fix bugs, etc.
You don’t want to create a lot of technical debt when you’re building your product, so it needs to be well architected. These are all things that a software developer learns in Software 101. Correction: software developers are taught that in Software 101. It turns out that sometimes it takes a lifetime to truly learn some of those lessons.
How do we make sure that we accomplish this in a code review? And why are they so hard? Well, like many things in life, it all starts with priorities. In many code reviews, the priorities are never defined and, by default, they’re never met. Here are some good guidelines you can use for prioritizing your code reviews.
- Architectural Issues with Integration
Most software isn’t monolithic anymore. Whatever you’re writing will need to integrate with other parts of a system in a way that allows you to accomplish your task while still allowing everyone else to accomplish theirs. Often when you’re writing a module, you are integrating it with other modules that someone else wrote. This means there is an especially high chance that you’ll break something in someone else’s code. A code review is the right time to catch those breaks, which means you should make sure the owners of the other modules are at the code review so they can help evaluate if there are any integration issues.
- Bugs
No one would bet QA that they couldn’t find any bugs in a large section of code that you wrote. Why? Because bugs are just a fact of life. If you’re writing software, then you’re writing bugs. Denying it won’t solve the problem. QA will. And so will a good code review! It’s hard enough to look at a piece of code and grok it. (Don’t believe me? How often have you heard developers say, “I’ll just rewrite this from scratch. It’ll be easier that way.”? That’s because it’s hard to understand someone else’s code.) This is the number one reason good code reviews are so hard to do.
In a good code review, you need to look at someone else’s code, understand it, and then find bugs. And not stupid bugs. You need to find the important bugs, the ones that are hard to find because really smart developers were already looking for them. My recommendation: examine the code beforehand so that when you walk into the code review you have a good understanding of what’s being written and can ask good questions.
- Modularity, Code Reuse, and Good Coding Practice
Now, this is the part of a code review that draws on Software 101. It is very important. However, we must consider it in context; if the code doesn’t work (i.e. has bugs) or breaks other code, then nothing else matters. I know, I know, some will say if the modularity or good coding practice isn’t present then we’ll never be able to fix the bugs, but that aspect remains academic if the code doesn’t work. It’s definitely very important, it’s just we must remember our priorities.
Making sure code is readable, abstracted correctly, takes advantage of code reuse techniques and the like is important to the sustainability of your code base. Remember, this isn’t just to look good in front of your peers, or to come up with sarcastic things to say. The purpose is to make our product better, save ongoing maintenance costs and improve our ability to add new features in the future.
We don’t have the space in this post to describe all the features of good code. We’ll save that for another post, another time (or another 50 posts, perhaps!). Suffice it to say, it’s important not to create technical debt – debt from hacking things together that will eventually need to be paid back with interest!
- Style Conformance
This goes hand in hand with point #3. Style conformance is an important part of good coding practice. However, setting everything to “high priority” is the same as setting no priorities at all. Some companies have great coding standards, while others have pedantic standards that can impede good coding practice. Additionally, this tends to be syntactic. We make sure to review for style conformance at Geisel Software, but this is much easier to spot and fix than priorities #1-3, which deal more with the nature of the code.
- Anything Else of Importance
This is a bit of a catch-all. Maybe there are things that aren’t necessarily in the style guide that we should conform to because they contribute to a code philosophy. Or perhaps our client has asked for something to be done in a specific manner. This is a good check and balance for things that don’t fall neatly into a defined category.
Let’s review our code review priorities:
- Architectural issues with other modules. Will the way this is developed be an issue for anyone else’s features/modules/implementation?
- Bugs.
- Modularity, code reusability, good coding practice.
- Style conformance
- Anything else of importance that pops out at you.
- Grammar and punctuation.
Oh, we seem to have skipped “Grammar and punctuation.” What a gross oversight! Um, not so much. The least important thing in a code review is grammar and punctuation. BUT, it is the easiest to spot, so it’s usually the first thing to be criticized. At Geisel Software we have a rule for code reviews: If your first two comments in a code review relate to grammar or punctuation, you are not permitted to speak for the rest of the code review. Let that sink in for a minute.
It is so easy to make comments about grammar and punctuation – even if they’re already right! These are the easiest errors to spot, but the least important to writing good software. This is one of the keys to why code reviews can be so tedious and useless. It takes a lot of time to really understand the code, its purpose, and why certain design decisions were made. If we’re all forced to listen to you recite your favorite passage from The Chicago Style Manual, we’re going to lose our minds before we get the chance to make a great bug find.
So, what about those rock star reviewers?
I’ve worked with a few people who could regularly spot really nasty bugs in code reviews. A huge part of what made them so successful was their ability to focus on the areas of the code that are the most complex. It takes brain power and energy to read complex code and really comprehend it. Hence, most people tend to avoid it. But, if you take the time to focus, avoid distractions (like should there be a comma before “and”?), and strive to understand the code you’re reading, you can be a rock star reviewer too!
Author: Brian Geisel, CEO of Geisel Software
