Code Review: How to Take on Unfamiliar Code
It’s commonplace for developers to be shuffled 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.
Often times I will have only a rough idea of the overall scope and very little 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 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 reviewing would as well, but it might have been quite clear to the original developer.
Eventually this review will turn into regular old refactoring, but at the start it isn’t quite, since I don’t know enough to do much more than superficial changes. For an average project I try to keep to a 3 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 for small code I might get all 3 done in an hour. It’s all dependent on the state of the project, I’ve come across awful projects where I spend a couple days doing 8-9 passes! Let’s assume we are dealing with an average project, below I have outlined my standard 3 Pass code review process.
Fix simple things and quickly scan the code to get an overall idea of size and scope and a rough approximation of code quality. A lot of this is going to be removing cruft, not only is it good cleanup, 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 just try and understand the general purpose of functions and not exactly how they work, save that for future passes or more in depth review after.
- Fix simple formatting
- Remove unused variables
- Fix poorly named variables
- Fix bad ordering of functions
- Drupal hooks mixed or at the bottom
- Constructions, initialize functions not near/at the top
- Fix Whitespacing 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 TODOs 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. These also might be a bigger problem that might require a serious rewrite.
Catch missed simple fixes and get into the logic a little. I probably now understand the code a little better and can make some smaller 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 of overly large functions
- Fix any TODOs from first pass that I now understand
- Add additional TODOs for more complex issues I notice
- Improve my initial first pass comments and add more
By this time I’m probably starting to get a good grasp of the functionality, 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 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 TODO’s from previous passes I now understand.
- Adding TODO’s for more complex stuff that I am now uncovering.
I usually won’t do more than about 3 passes, by this point I can probably settle into specific sections and start fixing bugs, doing significant refactoring and adding new features. Now I feel pretty up to speed and can starting working on the project like I normally would.