The Code Review of Many Faces
Code review is a powerful tool for giving and receiving feedback. When used properly we can inspire and mentor others, while also making our codebase (and the world) a better place. I believe that a good code review looks at the code from different angles, and so I have developed the Arya Stark method for reviewing a code.
Arya Stark was a talented warrior from Game of Thrones. Later in the series, she becomes familiar with the faceless men and learns how to change her face too. When she puts on the faces, she could feel feelings, thoughts, and memories of the character which makes it much more authentic than simply wearing a mask.
So how can we review a code A la Arya?
If Arya were to review a code, I bet it would have been an excellent one. A good code review should look at the code from different eyes and try to bring different perspectives to the table. When asked to perform a CR, we should review the code multiple times, and Like Arya, with each review, we can play a different role.
The puzzle solver
In this role, we will pretend that the code is a puzzle and every line of code is a piece. We want to get the full picture by putting the parts together. In this stage, we will be seeking context inside and outside of the code like understanding the business requirements and the author’s intention until we have the whole puzzle solved.
This part can be overwhelming, but it doesn’t mean it should be skipped as it is a critical stage in order to give meaningful feedback. I often end up reading the same code 3, 4, even 5 times. I read until the entire puzzle forms in my brain.
To solve the puzzle effectively, I believe that code reviews should be done async; because it allows the reviewer to examine whether or not this code is self-explanatory. Code reviews are also better to be done in a code editor because it gives the ability to search for usages, follow functions and jump around; all in the quest for a broader context other than the lines of code that have changed.
The Detective
In this role, we will pretend there’s a flaw hidden in the code. In an effort to identify the blind spots, we will try to understand if the code we are reviewing is meeting the business requirements and accomplishing the author’s purpose. In this stage, we are on a hunt for bugs and wrong behaviors. We should aim to find edge cases and try to challenge the code we are reading.
In order to be the best detectors, we should make sure the code is fully covered with different types of tests. There is no reason good enough to skip writing tests. If due to time constraints, some of the goals are at risk of not being delivered, then the solution is not to skip tests but instead descope the deliverables.
Some guiding questions we can ask to help ourselves be the best detectors
- Does this code implement the business needs as it should?
- Can performance be optimized? Can calls be async? Can DB queries or structure be better?
- Is this code fully covered by tests?
The Artist
Like art, the “beauty” of code is subjective. Two code bases can have identical functionality, look entirely different, and be pleasing to different people. I have heard developers argue passionately for specific conventions, concepts, and languages. Essentially, they are arguing over taste. Again, a concept fundamental to experiencing art.
But there is something we can all agree upon, a good code can be read like a book. This is our role here as reviewers, to ask ourselves how this code can be more elegant and enjoyable. This is where all the clean code skills we acquired can come in handy.
Leading questions
- Can variables and function names be more descriptive?
- Are the functions and classes small enough to follow?
- Can we avoid comments by providing more context in the code itself?
The Architect
Solving problems with code involves an immense amount of creativity. When a developer architects the structure of a program and infuses each step of the process with specific design choices, they are being creative. We should aspire that this new piece of code will improve the codebase. Like building a tower, we want to make sure that the foundations are strong and will not collapse when we will build the highest floor.
In general, I do think that design issues should be addressed in the design review. Although, there are low-level design issues that can only be discussed in the code review stage. This might be a bit of a conflict zone because it can yield long philosophical discussions, so bear in mind that there is no absolute truth in system design.
Leading questions
- Is the code reusable and follows the DRY principle?
- Does the code follow the single responsibility principle?
- How can we make this code more testable?
The Mentor
A good mentor or teacher is not only there to find mistakes, but also to encourage good work. This is our place to shout out for nice code changes or out-of-the-box thinking. Remember that receiving a code review can be sometimes an unpleasant experience especially in a new environment. Code reviews have a better likelihood to inspire others when we not only look at their mistakes but also appreciate their hard work.
So how can you be a better reviewer?
- Adopting the Arya Stark code review style, try to leave one comment from each role.
- Quality over quantity. The following chart illustrates different comments and their reward and conflict potential. I believe, most of the comments in a valuable code review should be focused within the high reward zone; favoring those within the low conflict zone.
- Go over your review and ask yourself if you would have been happy with receiving this review, if not, change accordingly.
- Ask for some feedback about your code review.
Remember that the main goal of a code review is not only to improve the codebase over time but is also to provide value to our peers and help them grow. Think about the good reviews that you have received in your career and how much they help you to be more professional.
Wouldn’t it be nice if you could provide this value to other developers too?
