Code Review (Week 13)

Code review is a practice in which software developers inspect and comment on code changes made by other developers before those changes are "accepted" (which might mean different things). This is a common (and common-sense) practice, but it can be difficult to do well.

Mechanics

In general, code review just means once developer inspecting the code of another developer, usually before the code is incorporated into the final program and distributed to users. The goal is to improve code quality (and readability) and reduce the number of bugs that actually end up in front of users. Additionally, code review can help a team maintain architectural and design principles. However, different organizations implement this idea differently.

One common pattern is to require all code merged into a repository to be reviewed by someone familiar with the project. This is very common in the open source community, where a "maintainer" (someone who regularly contributes to a project) will review contributions and decide which ones are ready to merge and which require rework (or possibly aren't suitable at all). In some cases, more than one maintainer must approve of a change before it can be merged.

Another common pattern is for a more experienced developer to review code written by a less experienced developer or an intern. This pattern is used to improve code quality as stated above, but it also helps the latter developer build good habits.

Strategies for Success

Code review can be a contentious thing. Developers have pride in their work, and there is often no "right" way to write a program, meaning many code review discussions involve subjective discussions. However, a few strategies can help keep the peace.

First, teams should reach agreement about as many "subjective" issues as they can in advance. For example, which style guide should we follow? Do we prefer designs that use traditional OO or a more functional approach? Do we write documentation for every method and field? What about private methods? How should we use code comments? These, and dozens of other questions are important to answer up-front so that they don't have to re-debated often during code review.

Next, it is important to keep the tone of code review friendly, or at least professional. Keep in mind that the goal is to help everyone who has to work in the codebase in question, not to win an argument or impose your will on the project. Aside from conventional advice on being nice (the golden rule and such), there are a couple of specifics to stay away from. For one, always explain yourself. Comments like "wrong, fix it" have no place in a code review. Additionally, it is important to separate the person from the code. Don't make code review about the developer, or his or her perceived flaws or habits. Focus on the code. If someone consistently makes a particular mistake, try to help them become better at what they do outside the context of a code review.

Finally, be consistent. Code review is only helpful if it applied consistently. Once teams, or a codebase, fall into bad habits it is very difficult to turn things around. Sometimes this can be frustrating. There is always a tendency to view code review and other practices (like testing!) as an inconvenience, and perhaps sometimes it really is. But if your goal is to deliver software that works, it's an inconvenience you've got to learn to live with.