Why Code Reviews?
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
- 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
- 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.
- to get a second or third pair of eyes to find bugs
- to improve consistency and quality of code in a team
How to Code Review
To prepare for a code review as a reviewee, it is best practice to briefly document:
- what your code does including what bug it fixes or what new functionality it introduces,
- what tests you have done on it and the outcomes
- how to run it if any nontrivial dependencies or build/startup instructions
- 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
As a reviewer, here are some basic things to check for:
- 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?
- Are there race conditions in the code?
- Does the code match up with dependencies and client code in terms of boundary cases?
- Are there obviously asymptotically inefficient loops and other blocks of code? How can they be improved?
- 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.