Code reviews are a key way for developers to find bugs, but many development teams fall into similar bad habits.
Code reviews are meant to be a systematic examination of computer source code, intended to root out mistakes made in the initial development process and improve the quality of the software being created. However, several common mistakes arise that make the peer review process less effective, Cameron Presley, a software engineer at SentryOne, explained in a recent session at the 2019 CodePaLouSa conference in Louisville, Kentucky.
Code reviews typically fall into one of two poor patterns. The first involves the reviewer not making any changes: "When there are no comments, that should terrify you," Presley said. "It leads to apathy—if you're rubber stamping, why do it at all?"
SEE: How to build a successful developer career (free PDF) (TechRepublic)
The second is when a simple set of changes turns into a long, drawn-out process, when quick changes turn into inefficient meetings with too many people involved to actually solve problems, Presley said. "It's exhausting, and a waste of time for you and lots of other people," he added.
The simple goal of code reviews is to find bugs early on in the process, since bugs cost more the later they are discovered, Presley said. Several case studies back this up, he explained: For example, IBM found that each hour of code inspection prevents about 100 hours of related work, including support at QA. And after introducing code reviews, Raytheon reduced its cost of rework from 40% of the project cost to 20% of the cost, and the amount of money spent on fixing bugs dropped by 50%, Presley said.
The nuance of coding comes from experience, Presley said. Code reviews are a low-risk way of learning more about a system, and picking up domain knowledge, he added. The process can also teach you how to better explain and teach things to others, which will help in any developer's own career development.
Here are four common mistakes developer teams make with code reviews, and how to fix them:
1. Trying to review too much code at once
Developers should review no more than 200 to 400 lines of code at a time, according to a report from Smart Bear. After that point, the effectiveness drops significantly.
Go no faster than 500 lines of code an hour
Work for no longer than one hour at a time to avoid losing focus
Break up massive features with thousands of lines of code into smaller chunks
If embarking on a large feature, start doing code reviews about 30% of the way through to see if there are any major flaws, and if the approach is correct
2. Getting opinion-based feedback
This type of feedback usually begins with "That's not how I would have done it" or "That's interesting," Presley said. However, the goal is not to have everyone write code exactly the same way, or think about problems the same way—the goal is to solve the problem. "You want people thinking from different perspectives, and diversity on the team," Presley said.
Frame suggestions as when, what, and why. For example, instead of "Never return null when retrieving a list of records from the database," say "When returning records from the database and there aren't any, we should return an empty list because all of the list methods work on an empty list, and we don't have to introduce error handling code." Basically, instead of saying "Never do this," open the discussion by saying "When this happens, we should do this thing, for this reason," Presley said.
3. Giving harsh feedback
Instead of criticizing someone's work harshley (for example, saying "This loop is garbage, what were you thinking?", rephrase your feedback (for example, saying "I'm having issues understanding this loop, can you walk me through what it's doing?", Presley advised.
Reinforce joint ownership. It's rare to work as the sole developer. Rephrase your feedback to use "we" instead of "you." "The developer is not the problem, the code is," Presley said. "This puts the pressure off them."
This situation involves one person doing all of the talking during the review, and the other person writing notes or just listening. It often arises when a senior developer is doing a code review for a junior developer, Presley said.
Emphasize collaboration, and the idea that everyone in a code review is equal. "If you don't understand my idea as the leader, call me out on it," Presley said. "You have to foster a culture of safety so everyone is comfortable speaking up."
Share domain knowledge, so everyone can cover for everyone else, even if someone is out or leaves the company.
For more, check out How to become a developer: A cheat sheet on TechRepublic.
How to become a developer: A cheat sheet (TechRepublic)
Implementing DevOps: A guide for IT pros (free PDF) (TechRepublic)
Telephone interview cheat sheet: Software developer (TechRepublic Premium)
Programming languages and developer career resources (TechRepublic on Flipboard)