As software developers, we strive to write code that’s readable, maintainable, logical, and functional. While this can be a daunting task, there are tools and processes that make it easier to achieve these goals: version control systems facilitate code and feature management; IDEs help us find and fix syntactic and semantic errors, even when using dynamic languages; testing frameworks help us prove to ourselves that our code actually does what we expect it to; and continuous integration helps us automate the creation of builds and the deployment of our finished product.
Ultimately, while these tools can be quite helpful in ensuring that the code in question works as expected, there’s no better way of verifying that code is readable and maintainable than getting it in front of other developers (that may someday be charged with its maintenance!) for feedback. Code reviews provide a number of benefits to teams; in addition to helping to find and fix bugs before they make their way into production, they allow developers to learn more about systems they might not be directly working on, and serve as jumping-off points for discussions about best practices that might otherwise never transpire.
If you’re thinking about instituting code reviews as part of your team’s processes, here are some points to consider:
- Use tools to make it easier. Chances are pretty good that whatever version control system in use by your team supports some way of reviewing code. At NPR, we use Crucible to create, comment on, and moderate code under review, and GitHub provides great tooling for reviewing pull requests.
- Reviewing code thoughtfully takes time. It’s easy to spend 5 or 10 minutes looking at a chunk of code and leaving superficial comments questioning variable names or codesmithing conditional blocks into ternary expressions, but it generally takes a bit longer to understand the greater structure of a new feature or system well enough to give constructive feedback. Our dev team dedicates one day per 2-week scrum cycle to code reviews, and everyone from product to design understands its importance and gives us the time to make sure it’s done right. (N.B. I don’t mean to imply that variable names and coding in the small shouldn’t be discussed as part of code reviews, as such considerations are also critical to readability and style.)
- Choose review material carefully. It’s very possibly the case that your team writes more code in a cycle than can reasonably be reviewed. As such, it’s important to use your code review budget judiciously when deciding what code merits review and what does not. (Ideally, no code would make it to production without being reviewed, but depending on your team’s size and resources, that may not be practical.) There’s no hard-and-fast rule about what code should be reviewed, but work that falls in to any of the following categories is probably a good candidate:
- The creation of a new feature
- A large-scale refactoring of an existing feature
- Code written by developers new to the team
- Code for systems with which most of the team is unfamiliar (this may be more about knowledge-sharing than the quality of the work under review, which is fine)
- When appropriate, make time for design reviews. Before embarking on the construction of a new major feature or system, it’s appropriate for the dev or project team doing the work to put together a high-level design document outlining their intended technical approach. This doesn’t have to be hugely elaborate — as little as a few paragraphs, and possibly a diagram or two, may suffice — but it’s very helpful as a sanity check. It’s terrible to find out in the context of a code review that you’ve wasted a week by taking a completely backwards approach to whatever you’ve been working on, and design reviews help in avoiding such situations by validating the overall plan before a single line of code is written.
- Code reviews > no code reviews. Don’t let the perfect be the enemy of the good. If your current workflow doesn’t include any sort of code reviews, find some way, no matter how small, to let your developers review each others’ work. The benefits of establishing a shared sense of ownership and understanding of the finished product far outweigh whatever process or tooling-related considerations might be holding you back.
I’ve actually found code reviews to be so helpful in improving my ability to critique and write clean code that I’ve incorporated them informally into my own coding practices. Rather than waiting for “official” team code reviews to take a good look at whatever I’ve written, whenever I’m about to commit or push code, I take a few moments to review what I’ve done to make sure that everything is as it should be. It’s amazing how many errors I’ve caught by doing this, even when I’m writing a tiny amount of code or am sure that I got it right the first time. It also requires very little effort; I’ve aliased ‘gdx’ to ‘git diff | gitx‘, which means that a nice visual diff of the changes I’m about to commit is only 4 keystrokes away. At this point, quickly reviewing my code prior to committing feels as natural as proofreading email prior to sending, and the return on investment is absolutely worthwhile.
