How to do a code review, Part 1

Code review is a practical and important skill that truly distinguishes the highly experienced engineer. Knowing what are the common pitfalls, where to look for them, and how to understand someone else's code quickly is an invaluable skill to have. It is also a skill that is rarely taught in school. To really ramp up your programming skills, get out there and read other people's code. Reading high quality code is a good learning experience. Occasionally reading poorly written code is also insightful. What are the most common errors and where do you find them? Through code reviews, software engineers can spread knowledge through their teams as well as improve consistency and code quality. For the interviewee, you have to review your own code, so it helps to know what are the things you need to double check. One great way to evaluate a team's culture is to probe and understand their code review practices or lack thereof. Below, I will cover some of the best practices and provide a few pointers on where you can get started.

One important thing is that all parties have to be conscientious to make code review work and to avoid the whole process turning into an adversarial process, which it definitely can. Code review shouldn't be something dreaded by both reviewer and reviewee. It ought to be a process beneficial to both.

Fundamentally, reviewing code is for the purpose of

1. the reviewer understanding how the reviewee's piece of code works (so more than one person in a team will know how to maintain, extend, and support that code) and for the reviewee to understand any potential unexpected interfaces with other code
2. for the reviewee and reviewer to exchange ideas about what is the best way to do things in code. The knowledge transfer is bidirectional. It both the experienced reviewing the junior and the junior reviewing the experienced.
3. to get a second or third pair of eyes to find bugs
4. to improve consistency and quality of code in a team

A team needs to have some ground rules for code review. Team mates should agree whether code style is in scope. When they are in scope, they can be identified as a nitpick (a nit).

To prepare for a code review as a reviewee, it is best practice to briefly document:

1. what your code does including what bug it fixes or what new functionality it introduces,
2. what tests you have done on it and the outcomes
3. how to run it if any nontrivial dependencies or build/startup instructions
4. where to find your code if you aren't using a nice code review tool, you should package up your code as a diff using a diff tool

If at all possible, you should keep the unit of code to review to a manageable size, say 200 lines of C/C++ or Java code. The code review process should be a conversation. The reviewer supplies feedback, which he or she will expect you to act upon in a timely way. You should agree ahead of time roughly how long before a reviewer will get to your code after submission. Once you received the feedback, let your reviewer know when you'll address each of the points. Again, a code review tool can be helpful here to keep track of feedback and fix status. While you address the feedback, it may take a few times back and forth before you and your reviewer arrive at a mutually acceptable resolution. Both reviewer and reviewee will have to be patient here.

As a reviewer, here are some basic things to check for:

1. Is there a real potential for an unhandled error or uncaught exception from a function call? The larger question is how error handling is done. Are errors being handled gracefully and in the right place?
2. Are there race conditions in the code?
3. Does the code match up with dependencies and client code in terms of boundary cases?
4. Are there obviously asymptotically inefficient loops and other blocks of code? How can they be improved?
5. What happens to this code if I use it in another relevant context? Do all the assumptions still hold?

Github is a veritable treasure trove of public code reviews. One learns best when one is the reviewee or reviewer, but it can help just by reading other people's reviews. Pay attention to what the reviewer is looking for.