Code Reviews: Write Better Code Overnight

In: Columns > View Source

By Garrett Dimon

Published on July 3, 2006

It’s a common practice among good teams of programmers, but I rarely meet front-end coders who know what code reviews are. Few of those who do know what they are organize them regularly.

What’s a code review, you ask? It’s a time of great rejoicing, when a team of people who write code get together and constructively review each other’s work in order to share, learn, and teach better programming habits and techniques.

Why Bother?

Before we get into the details of how they work, it helps to understand why code reviews are so valuable. On the surface, you might think it’s because they help uncover bugs. But when you look deeper, the value goes far beyond that.

First of all, code reviews get team members involved in different parts of the system than they’re used to. Team members may be so heads-down and intimately involved with one part of the system that they overlook a simple solution that is obvious to someone not as entrenched in the project. Sometimes, two team members working on similar modules discover, through a code review, that they can share more code than they previously thought. This helps reduce redundancy and keeps the team educated about what everyone is doing.

Another benefit is that code reviews encourage consistent coding standards and help ensure that everyone is on the same page. Coding standards may not seem that important, but cleaner code makes it easier for someone else to come in and start working with it. Additionally, coding standards can help prevent common logic errors or other simple mistakes.

One of the more obvious benefits is that your team as a whole will create better code. More sets of eyes increases the quality of the code, so your project will have fewer bugs and be easier to maintain over time.

These advantages aside, the single biggest benefit of code reviews is the opportunity for education. Senior team members can impart their wisdom and insight to junior team members. Juniors can offer senior coders unbiased and fresh ideas overlooked by someone more set in their ways.

How Does It Work?

Code reviews are really pretty simple: A couple of quick steps and a focused meeting, and you’re done. You may want to make adjustments for your situation and environment, but here’s a basic set of guidelines to help you get started.

The most important thing to remember is that code reviews must be a priority for everyone. As soon as the team thinks of them as something that happens only if there’s extra time, you may as well forget about it. Code reviews are a long-term investment in a team, and it’s often easy to put them off because the benefits aren’t as tangible as getting a few extra hours of coding done.

The second most important aspect is that of respect. In any code review, the goal is growth and education, and it’s important to remember that the code is being reviewed, not the individual. This is important for both the reviewer and reviewed. The reviewer should be critical, but considerate, and the reviewed should remember that the purpose is to help them improve and graciously accept feedback.

Finally, it’s imperative that everyone on the team—even senior team members—reviews code and has their code reviewed. Code reviews are a team activity that helps everyone improve, but if only a few team members participate, the results won’t be nearly as impressive.

Code reviews begin the same way as any other meeting. Plan a time for it, and make sure that all of the necessary people can attend. Remember, If you don’t plan the code review, it won’t happen. Once that’s taken care of, the individual whose code is being reviewed prints out copies for all of the reviewers 24 hours before the meeting. Generally, it’s best to review an entire module of code. For front-end code, a few pages of HTML and CSS is sufficient. Don’t forget to make sure that your printouts have line numbers so it’s easier for everyone to follow along during the meeting.

At this point, it’s the responsibility of the reviewers to read through the code and mark it up by hand. In the case of HTML, CSS and related code, this means nitpicking semantics, indentation, capitalization, ID and class naming conventions, among other things. The goal here is to make the code more concise, readable, efficient, flexible, and effective, and any recommendations to those ends are welcome.

Now it’s time for the meeting, ideally around a conference table. You’ll need a moderator who guides the meeting along and keeps everyone on task. The moderator is also responsible for making sure that the meeting stays constructive and focused on reviewing the code rather than attacking individuals. The moderator will also be the first to comment and say where he or she has the next comment for a specific line, and if someone else has a comment before that line number, they can jump in and provide their feedback.

This process repeats throughout the chosen section of code until everything is wrapped up. The reviewers can provide their copies of the marked up code to the reviewed, and the meeting is finished. It’s now up to him or her to sort through the feedback, implement the necessary changes and get back to work.

What to Look For

So now that you know why and how to do code reviews, let’s talk about where to focus your critiquing talents. You might think it’s easy to critique someone else’s code and find simple little things to improve, but it’s often much more challenging than that.

Style is the easiest—but least important—area to focus on. It’s obvious if someone is conforming to team standards or best practices (like whether they’re using too much or too little shorthand, or employing poor naming conventions). However, that’s just where better code starts.

In the case of HTML, semantics are an important area to pay attention to during a code review because there are numerous ways to mark up elements, but one or two ways that are clearly better. Similarly, with CSS, the options for organizing the code are virtually limitless. Selectors can be written differently to accomplish the same thing with more or less flexibility depending upon the future plans for a given page or site. Additionally, in some cases it makes sense to use shorthand, while in others, it may be better to use more explicit references for properties.

For instance, I was at a code review recently where a team member chose to use RGB values for his colors in CSS. This isn’t a bad thing, but his reasoning for using this methodology was that he simply didn’t understand hex values. Once we figured that out, we held a quick impromptu session explaining how hex values worked, and everybody gained some additional insight. Remember, the single most important aspect of code reviews is education.

Of course, you also want to be on the lookout for areas that could be written with more clarity or shared by other team members as well. This is one of the more difficult areas to recognize, and the senior team members play a much bigger role here. Every team has one or two people that have a much higher level of general understanding of the project, and those are the team members that can recognize and expose when there could be more shared code or collaboration.

Summary

Code reviews aren’t necessarily easy or convenient, and the long-term value isn’t always readily apparent. However, as a practice, they can dramatically improve the skills of your team and even immediately improve the quality of your team’s code. All things considered, the effort is minimal, and the rewards are limitless. Give it a try, and I guarantee you’ll like the results.

Related Topics: Programming, Scripting, Critique