Code review: How to take on unfamiliar code

It’s commonplace for developers to move between projects, and it can be challenging to work with unfamiliar code. You’ve probably experienced one or all of the following:

• Taken on a project from another company
• Been asked to save an internal project from falling off the rails
• Added to a project team late

I’ve encountered many new developers who struggle and will spend a significant amount of time trying to get familiarized with the code and the project. I have documented a bit of the process I use when picking up or reviewing code I know very little about.

Three-pass system of code review

Frequently, I will have only a rough idea of the overall scope and minimal idea about how the site internals work. To get myself up to speed and start understanding the code, I do a simple code review and cleanup. This review helps me learn the code and make it easier to understand. As someone who didn’t write the code, this also makes me a good test for comment quality and variable and function naming. If I find it unclear, likely, someone else will as well, even though it might have been quite clear to the original developer.

Eventually, this review will turn into regular old refactoring. But it isn't quite refactoring since I don’t know enough to do much more than superficial changes. For an average project, I try to keep to a three-pass system (outlined below). I find I rarely get everything all the first go at it. I’ll probably spend an hour or more per pass, although I might get all three passes done in an hour for small code. It’s all dependent on the state of the project. I’ve come across awful projects where I spend a couple of days doing 8-9 passes! Let’s assume we are dealing with an average project. Below, I have outlined my standard three-pass code review process.

First pass

A lot of this is going to be removing cruft. Fix simple things and quickly scan the code to get an overall idea of size, scope and a rough approximation of code quality. It is a good cleanup, but it also means there is less code for me to try and understand, and I don’t have to try and understand stuff that doesn’t matter. It is helpful to understand the general purpose of functions and not precisely how they work. Save that for future passes or a more in-depth review after.

• Fix simple formatting
• Remove unused variables
• Fix poorly named variables
• Fix the bad ordering of functions
• Drupal hooks mixed or at the bottom
• Constructions, initialize functions not near/at the top
• Fix whitespace or lack thereof
• Remove commented-out code
• Remove useless comments
  • Ex. Set variable to 5
  • Ex. Loops through list
• Remove comments that do not match the code
• Add simple comments to areas I didn’t understand; I may even remove these again later.
• Remove abandoned debug statements.
• Add TO-DOs for parts you think need a more thorough review or where I see an obvious problem but you don’t know enough about the code to fix it yet. There also might be a bigger problem that might require a serious rewrite.

Second pass

Catch missed simple fixes and get into the logic a little. I probably now understand the code better and can make some more minor logic changes or refactoring. I can probably start to spot duplicate sections of code now that I have an idea of the whole project.

• Fix additional first pass issue I missed
• Clean up sloppy if statements
• Fix unnecessary sets, loads etc. in loops
• Simple consolidation of duplicate functionality
• Breaking up overly large functions
• Fix any TO-DOs from the first pass that I now understand
• Add additional TODOs for more complex issues I notice
• Improve my initial first pass comments and add more

Third pass

I’m probably starting to get a good grasp of the functionality by this time. I will start doing some more significant refactoring and probably adding or rewriting comments at this time and doing some serious work. I will be stopping on some sections and breezing past others now that I know more about where the problems lie. At this point, I try to understand how stuff works, not just its general purpose.

• Rewrite small sections for logic flow, think simple refactoring and smells
• Start moving some stuff between files now.
• Deleting extra code that never runs
• Further fixing TO-DO from previous passes, I now understand.
• Adding TO-DOs for more complex stuff that I am now uncovering.

Afterwards

I usually won’t do more than about three passes. Now I feel pretty up to speed and can start working on the project as I usually would. I can probably settle into specific sections by this point and start fixing bugs, doing significant refactoring and adding new features.