8 Things to Check in a Python Code Review
Code reviews are one of the most important activities in the daily workings of a software team. They are an opportunity for everyone to become familiar with the codebase. They help prevent siloing (one person becoming the only source of knowledge). And they are a great opportunity for beginning programmers to learn from more experienced programmers.
When I do a python code review, I’m looking for several things.
Here is a list of eight basic things I look for first.
This is assuming that the code was formatted (using black, yapf or something similar) and linted (it follows pep-8) before check-in.
1. Are there things that were left there from debugging or working out the code? Print and debug statements are surprisingly common.
#remove these:
print(response)
import pdb; pdb.set_trace()2. Are there constants in the code that should be declared at the top instead?
with open("config.json")
#instead declare them at the top:
CONFIG FILE="config.json"
with open(CONFIG_FILE)3. Are the names of functions and variables well chosen?
for ws in resp[“object”]
#instead:
for wheel_size in resp[“object”]4. Are there variables that are only used once (single use variables) that could be folded into the code instead?
up = last_name.upper()
if up in [“SMITH”, “JENKINS”]:
…
instead use it in the code:
if last_name.upper() in [“SMITH”, “JENKINS”]:
…5. Are the for and while loops written in a pythonic way? Could they be replaced by a list or dictionary comprehension? Keep in mind though that a list comprehension can easily get to the point where it is too complicated. In that case the for loop might be easier to understand.
6. Does code get repeated in different parts of a file, or even different parts over multiple files? If so, put that code into its own function and maybe even a separate file that can be called from multiple files.
7. Does the same data get passed around a lot? Maybe it would be better as an instance variable in a class.
8. Were parameters deleted in a function? Double check that those parameters weren’t important to other python functions or files. Do a search through the whole repo, or even through the whole codebase for that function.
These are basic things that can be examined without thinking too much about the code.
Beyond this there should be questions about the overall architecture of the code and how it fits into the project.
One book that I highly recommend reading is Refactoring by Martin Fowler. The code examples are in javascript, but the concepts are very useful for all languages.
