My focus is on making software, coaching Agile/Scrum teams, and being a husband and father--definitely not on blogging--so please do not expect very frequent posts here.

Thursday, August 15, 2024

Code Reviews Are Overrated (but not how you think!)

When I first began leading a "two-pizza" software team in 2007, our code reviews focused on code robustness and correctness. Our architect would choose code to print out--usually around 4-6 pages' worth each week. We would gather in a conference room and spend an hour scrutinizing, and ultimately arguing, about what in that code should change. A few months into the job, he turned over that duty to me and I knew both the printed approach and the goals we were pursuing had to shift! Since that time, tools have greatly improved, code review practices have become easier and more widely known, and the way we structure code is less novel and more relatable. On top of these improvements, developers expect to review code more commonly and with a much healthier attitude; the benefits are more intuitive to all. However, after doing code reviews on most days for the last 17 years, I have come to see that some of the supposed benefits of code review are overrated. Let's acknowledge the limitations of code review toward those goals and consider other goals for which code reviews might be the primary means to gain.

Most importantly, code reviews are not great at finding defects (bugs) in the code. I hesitate to even mention this first: indeed, my teams have used code reviews to find and fix thousands of defects in our code over the years--perhaps making code reviews worthwhile for this benefit alone. Furthermore, bugs found in code review can be fixed much more easily and cheaply than those found in any round of testing, or in the worst case, found by actual users in production. Nevertheless, if we relied on code review as the main way to prevent defects, our quality would dramatically drop. Most defects are simply not obvious enough in code reviews. We need testing in all its forms. We need mutual understanding of requirements, use cases, and/or user stories. We need collaboration and an ongoing conversation about what our software should do and what our users' experience should be. We need an overall team culture growing in software quality and technical excellence. There is not one way to prevent defects; we need all the ways. Certainly code reviews are one tool for higher quality software, but the answer to "why did this bug make it into production?" never comes back as "Eric reviewed the code poorly."

Now let me restate that overrated does not mean nonexistent; let me repeat that code reviews do often find bugs; they just aren't great at doing so. Much as a .300 hitter (that is, a 30% successful hitter) in baseball is an all-star, the value in preventing defects via code review is large despite its failure to detect them more often than not. Keep looking for bugs in your code review effort--just don't expect to find most of them that way.

A concept related to defect prevention is correctness. Does the code do what the author intended it to do? Code reviews are overrated in revealing incorrect code. After all, unless the review somehow expresses what the author intended separately from the code that implements it, the reviewer cannot tell where the intent of the code differs from the code. Reviewers often proceed assuming the best intentions of the author and also assume that the code written accurately reflects those intentions. If a reviewer encounters code they didn't expect, he might think "well, she thought this through and I haven't. She must be right." The wise reviewer will ask questions instead of assuming the best and use the review as a jumping-off point for discussion and collaboration, but no one will be able to question enough or in the right places all the time. Fortunately, there is a proven practice that is explicitly and mainly about checking the code written against the author's intention: unit tests are exactly what we need, especially when they are written side-by-side with the implementation code, either immediately after ("code-then-test") or before (e.g. via TDD). If you aren't using unit testing and instead relying on code reviews to ensure correctness, a lot of misunderstandings and mistakes will slip through to QA or beyond.

The flip side of defect prevention is the completeness and robustness of the code. Does the code do all that it should? Again, code review only examines what the code does. Code review isn't so good at flushing out what the code isn't doing but should be. A really good reviewer will have a strong understanding of the intent and end goals of the code under review and thus can and should question the author on what might be missing. However, sometimes the author is already the person who knows best about what code should be doing, and other reviewers might assume too much given the code they see. Again, use the review as a jumping-off point for discussion and collaboration. But even this healthy attitude will only get you so far. Code reviews are no substitute for a team-wide mutual understanding of the both the user's needs and the big-picture direction of the software effort. It takes many other practices to build the product right; these include product planning time, technical design meetings, pairing, prototyping, and frequent review/discussion with the product owner and users.

Code reviews aren't good at revealing UI/UX shortcomings because code doesn't show the interface or how users interact with it. Testing, especially user-acceptance testing, of any user interface cannot be replaced by code review. A team must engage in UI/UX reviews, both among the team and with their users or user representatives. However, I do highly recommend expanding the idea of "code" reviews to more than code: include mockup/prototype reviews or even reviews of nearly-finished screenshots. Reviewing a screenshot, a UI mockup image, or a photo of a whiteboard is far more valuable than merely reviewing the code that might generate such an interface.

Technical non-coders can't rely on code review to stay in the know. Architects and tech leads who require that all code changes be reviewed by them do their teams a disservice. They are hindering all three of Pink's factors of job happiness: they are blocking their team member's autonomy, preventing them from attaining mastery, and de-emphasizing the purpose of building the software product. At best, they make themselves a necessary impediment to getting the job done. At worst they become the lowest level of rubber-stamping bureaucrats. Instead, architects and tech leads should be sure they are occasionally authoring code just as they did as senior developers, and they should sacrifice time doing code review if necessary to achieve it. By all means architects and tech leads should participate in code reviews (and their recommendations should be valued by all), but the team should be primarily responsible for reviewing each other as peers. The team should be empowered to approve and complete code reviews quickly and to do whatever necessary to prevent code reviews from becoming any kind of delay.

Finally, code review is a poor substitute for cross-training, mutual team learning, and team-wide ownership of the requirements, code, and software deliverables. Code reviews are just too narrow to be our main teaching tool. Because teams that do code reviews well will tend to shorter reviews in higher quantity, any given code review will not expose enough of the software to generate the kind of conversations needed to genuinely spread knowledge around. We can hope that code reviews will help spur on broader conversations about our software, but we cannot depend on them as the main tool for doing so. Furthermore, the deep knowledge needed to grow as a team can only come from implementing or improving that software. Reviewing does not make the reviewer as knowledgeable as its author. Planning, team organization, daily scrums, pairing, and swarming can all help with this. These practices and more will automatically happen the more your team takes on a mindset of collective code ownership.

Thus are you discouraged about the benefits of code review? Don't be. The supposed benefits above can still be had, just not to the extent that you might have hoped for in code reviews We can gain some defect prevention, correctness, robustness, usability, and learning from code reviews; we simply cannot rely mainly on code reviews for these goals. Supplement code reviews with other practices tailored to these goals.

But I haven't yet written of the underrated first-class benefits of code review. Turns out there are many gains for which we can rely on code review--and especially on team peer code review. Compared with my team in 2007, today we review lots more code more efficiently and with different goals. I'll explain a few in my next posts!  Till then, let me know how I can help with your team's code reviews or perhaps review your code myself as freelance side work.

No comments:

Post a Comment