While beginning to explain code review to a customer recently they interjected a remark, “sounds expensive”. Thereby raising the question, is it worth it? While I don’t presume I can empirically prove to you that it is, I do believe I can convince you.
For those less familiar with code review, the next few sections provide some context about what code review is. If you’re already familiar with the subject, feel free to skip to the “why code review?” section.
What Is Code Review And How Does It Work?
Generally code review is a process by which software engineers review the code of their peers. It’s somewhat akin to an inspection. After an engineer completes the code for a given task they invite their peers to look over their changes. Each peer then takes on the role of the inspector, reviewing changes to each file modified by the implementer. Why? What are they looking for? What is the goal?
The inspector is looking for numerous things including, but not limited to, the following:
- Did the implementer adequately address the requirements of the task?
- Does the code achieve its goal in an appropriate manner, balancing elegance and simplicity?
- Is the code adequately tested?
- Will the new code be maintainable?
- Is the code adequately commented?
- Does the code limit or reduce repetition of code?
- Is the code adequately formatted?
Once the code is reviewed and the feedback is processed the code is approved and accepted.
When And Where Should Code Review Be Done?
Traditionally code review has been a team exercise in which every couple of weeks the entire software engineer team was corralled into a conference room. One by one engineers would showcase the most prominent parts of their work since the last review. While doing so, the rest of those in attendance would ask questions and make remarks regarding everything from impact to application architecture or performance to commenting and code styling. While this approach was an understandable starting point, over time it revealed several flaws which led to newer more efficient means.
Today most teams, including those here at Saturn Systems, use a software system which implements a workflow allowing for code review to be conducted in a more flexible and efficient manner.
Instead of teams huddling around a conference table, they conduct code reviews independently from their own computer and on a schedule more conducive with their own self driven process. This largely improves the efficiency and quality of code reviews by preventing participation domination, a common problem in the traditional group review model.
More importantly this process is more accountable. Because it is governed by a software system, it includes automated checks and balances which at best used to only be “enforced” via policy. In my experience it doesn’t matter how thorough or well thought out your code review policies are, if they aren’t accompanied by a system of accountability it’s too easy to avoid or ignore them. An effective system requires all code to be reviewed, not just the parts an engineer wants to show. Thus preventing any code from being “snuck in”.
If your team is still adhering to an older approach to code reviews it’s worth considering modern alternatives in the interest of greater efficiency and effectiveness.
Who Should Participate In Code Reviews?
This is a question that is sometimes debated, or sometimes results in exceptions for “special developers”. Those whose legend and mythical abilities rival Nero of The Matrix. The rationale can vary, but generally stems from a sense that the engineer’s prowess negates the need for review or that their time is too valuable to be expended in this manner. Unfortunately, such rationales are generally short sighted. While a single engineer’s productivity may be protected by this methodology, ultimately the teams’ productivity suffers.
Ultimately, based on my experience its best if code reviews involve all developers. That said, it needn’t be every time. By that I mean that it isn’t necessary that every set of code be reviewed by every developer. Ideally, especially for larger teams, there will be two or more individuals to serve as a mandatory reviewer combined with a minimum number of approvals by the remaining team members. When a healthy balance is achieved code review is accomplished without hinging on a single individual team member.
Why Code Review?
The benefits to code review are numerous. Chiefly it provides broad accountability and shared responsibility. It serves as preliminary quality assurance as developers contrast implemented code with specified requirements and inspect code for flaws. Further it provides an opportunity to flag and address poor architecture design choices before they become systemic cancers in a codebase.
Addressing these items early through review provides quality training to developers, teaching them proper ways to solve problems and why those methods are important. Through code review developers also cross train each other on the entirety of the application so that all developers are better prepared to work across the application versus being siloed into specific aspects.
Another significant, but often overlooked benefits to code review is its impact to mentoring. When done correctly code review is like a rising tide elevating all ships, everyone’s skill is honed. It provides the ideal space for more experienced developers to little by little impart their wisdom to less experienced developers. Reciprocally it also arms those less experienced developers with knowledge to hold the more experienced developers accountable. I’ve often received regurgitated feedback from other developers that I had once imparted to them but forgot or neglected in other instances. Additionally, it can also (sometimes frequently) lead to the less experienced schooling the more experienced with new tricks.
Finally, code review distributes responsibility, so that liability is shared by the team when something goes wrong. Since the whole team participates in code review and approval, everyone has the opportunity to catch defects and when something is missed, effort can be focused on solving problems as opposed to assigning blame. All in all, when done properly, these perspectives combine together to create a positive and encouraging process that bolsters the confidence of each team member and ultimately the quality of the product.
So, Is It Worth It?
This leads us back to our original question, is it worth it? As my customer aptly pointed out, code review certainly does appear to carry with it a significant effort and therefore expense. Of course, the savvy among us will recognize that expense is relative. Thus, I’ll conclude with a tale of two brief tales.
The first tale is that of a first-year developer who picked up a basic clean-up task to rename an API property. In so doing they reasonably performed a search and replace for all instances of this unique property name, committed the changes and issued a code review. Though the continuous integration server picked-up the changes and successfully built the project. Even with decent unit tests and code coverage, a flaw was not detected. The search and replace included changes to the database migration history which effectively broke both the migration history and database instantiation.
Regrettably this change also went undetected through code review and remained so until a later date when the database needed to be recreated. At which time a quality assurance engineer encountered an application failure with a general error message. With no indication as to the originating issue or code they recorded a bug after doing the proper analysis to capture the contrast between the expected and actual behavior.
Finally, the bug was scheduled for development at which point the assigned developer began attempting to recreate the bug and reviewing any application generated exceptions. After initially discovering a strange error concerning a non-existing column name, the developer spent a fair bit of time researching and debugging the error before finally discovering that somehow the historical representation of the property name had changed. All told, by the time a fix was implemented, committed and re-tested the total effort to overcome the original flaw surpassed 8 hours of collective effort.
The second tale begins in a similar manner, only this time it ends abruptly. During code review, the engineer who fixed the previous defect recognized the phenomenon and with a brief comment the engineer was able to educate the entire team, most notably the implementer, to watch out for this occurrence and the flaw was quickly remedied. Less than one hour of total team effort in code review subverted the re-occurrence of 8 hours of debugging and re-work.
So, is it worth it? Are you convinced? After all the initial code review did not succeed, which, as it turns out, is the entire point, the lesson had not yet been learned. Once learned, code review effectively short-circuited the path to circumvention. Point being, code review isn’t perfect. It doesn’t catch everything. But, it does help us collectively learn from our mistakes, which is its true virtue. So, while a single tale of two tales is undoubtedly too minimal of a sample size, no sample size (no matter how large) will lead to empirical conclusion without debate. It does leave us with the appropriate application of wisdom from leadership author Kenneth H. Blanchard, “None of us is as smart as all of us.”