Defect Density

Defect density (DD) describes the number of faults occurred per lines of code (LOCs). It can be used to have a really high-level view of the status of the code base.

Indeed, as we early described in this book, just counting defects is not enough, and each of them should be properly prioritized.

As an example, consider the case of a security code review, where different vulnerabilities might be discovered. Not all of them are created equal. Nor their impact to the business. In this case, defect density would give a quantitative view, not a qualitative view.

Defect Removal Effectiveness

Defect removal effectiveness (DRE) , also referred to as defect removal efficiency, expresses a measure of effectiveness of solving issues during the software lifecycle.

It is generally useful to determine product’s healthiness and can be applied to code review. DRE can be defined in terms of reviews as follows:

(100%)

Review Metrics

Inspection Rate

Inspection rate (IR) gives insights on the velocity of the review process. Indeed it is defined as

where LOCs is the number of lines of code of the analyzed component and hours is the amount of hours spent in performing the review.

This metrics is oftentimes used to get an understanding of the quality of the performed review. A lot of opinions have been expressed about a guess of which IR values express a well-performed review.

I personally do not buy into it as it is and I would still contextualize this type of information and perform an educated validation.

Code Coverage

Code coverage is the degree of code being inspected. In code reviews, the expected coverage is 100%.

But remember: always break down complex tasks into smaller, more manageable problems. It’s true that complete code reviews should be regularly included into the development process. However, better to start reviewing code as soon as possible after a change is performed or a new feature is added to the codebase (e.g., peer reviews). It will make things easier when the complete review will be due and it will help in achieving better code on the go.

Defect Detection Rate

Defect detection rate (DDR) expresses the velocity of finding defects. It is computed as

The approach for looking at this metric is equivalent to the one used for inspection rate. Use it, but do it with caution and evaluate it depending on the context. Certain times code is so bad that defects jump out from everywhere, other times it is already in a fairly good shape (in which case, high five!).

Clear Review Goals

It is important to clarify what the objectives of the review are. This will help you to be on track during the process. The objectives will depend, between all, on environment, stage of the project, requirements, scope, and technologies used.

Stay Scientific, Stay SMART

Avoid recommendations like “a faster DB/component/similar would be cool.” Argue what is actually needed (measurable), why, and the impact on the project.

Use the SMART approach. Always be specific with the defects, and have measurable and achievable suggested improvements. Reviews insights should be relevant to both the type of review you are performing and the problem to be solved. And they should be time bounded by means of having specific actions to be taken to remove defects with a specific deadline.

Plan Ahead

Checklists Are Simply Not Enough

Checklists are very useful as guidance during reviews. However, just walking through a checklist is not enough, and other methods, including scenario-based architecture reviews, should be considered. This would enable a well-rounded assessment.

That’s a Trap

One of the biggest traps that threaten code reviews is the risk of allowing the process to become judgmental. This approach toward code reviews creates a toxic work environment, destroys team morale, and hinders the production process.

Please, avoid approaching reviews as a way to challenge colleagues. Easier said than done, I get you. Hone your communication skills, if you have to. Almost everybody has rolled their eyes while looking at some not-that-cool code before, but resist the urge to let it cloud your review. Please, go into the process with a positive attitude: help, teach, and reward success. Don’t jab your finger at other people’s flaws. Constructive feedback goes a long way. Reviews can and need to be a team building activity and a learning experience.

I get it. I love beautiful code, but let’s keep competition out of the door. Code reviews are not an unhealthy challenge on who writes the most beautiful code in the team.

The second trap is the risk of making the review personal. The defect is in the code; you are not scanning colleagues. The code does something in a wrong way; it is not about Mark or Mary. Thus, be careful to communicate defects in the proper way, nothing personal.

The third trap, nonetheless important, is about being frustrated with a tedious review process.

Let’s consider reviews performed before each merge request. How many of you have been stuck waiting for a colleague to review the code? How many waited too long to perform a review?

Manners

First and foremost, if you volunteered or you have been asked to perform a code review, commit to it. Do it accurately, walk through all the steps, and don’t just glide over it.

Hence, do not just assume that the code works, not because it comes from a more senior peer. Remember! We are not scanning people. You trust your colleagues for doing a great job, and that’s amazing. But it does not imply that you do not have to do a great job yourself. Rebuild and test the code. How many of you found yourself in the situation of ending up with broken tests? Maybe because of a change in an interface? Fully commit to the process and do your best.

If the code integrates well, do not stop the process here. If a new functionality is developed, inspect for documentation and additional tests needed. Is the committed code accompanied by proper documentation and use cases? If not, it is the right moment to raise the necessity and fix it.

A common dispute is about deciding if code reviews include testing or not. This is especially true for teams where a defined distinction is made between who does reviews and who cares about quality assurance (QA). My thought is already clear. During code reviews, some testing needs to be done. Catching bugs and flaws is not the responsibility of a single person or team. The sooner they are detected and fixed, the better; period.

If you found the code hard to read and understand, do not make biased decisions. Nor the perception of your own or someone-else seniority should have any impact on the review. Do not be scared to ask the programmer who wrote the code for a clarification about the context and the approach they used to implement it. Readability is a huge principle in good code. The worst that can happen is that you learn something or something was missing and the code improves. Not that bad, isn’t it?

Finally, after the review, do not skip reviewing again. Does the new code incorporate all the required changes? Rebuild and retest. Check again for documentation and additional tests. All of the steps should be made. Second review might be slightly faster, but cannot be taken at ease. Reviewing can be daunting, but it is worthwhile in the end.

Quality Means Faster

Especially in Agile teams, adding code reviews looks like a lot of effort that adds up to the already full Sprints. Sure enough quality code comes with a price. But quality code also implies a faster development process. If the code is incrementally constantly improved, developing new features is faster. With less bugs, anomalies, and with greater readability and usability, the team is allowed to focus each and every time to keep on providing value (actual functionalities) rather than being constantly stopped because of unclear, unstructured, or buggy code that needs to be fixed first.

In other words, it helps in reducing firefighting. Firefighting happens when different priorities take over on each other causing a lot of context changes.

As shown in Figure 11-2, when firefighting happens, the engineer needs to eventually interrupt the development process to fix the defect and test again and only after they can go back to developing the actual feature. This slows down the process and reduces productivity.

Distributed Teams

Any engineering manager knows how tough it can be at times to manage a team especially if it is geographically distributed. Communication is slightly more intricate than “normal” teams.

To deal with it, a lot of meetings are added to the daily work to facilitate communication. Anyway, as developers, we communicate with both verbal language and code. Thus, you should not only care that all the interacting team members share a common language (e.g., English), but they should also share the same coding process and (good) practices.

Show Me the Math

If I did not convince you so far, let’s gather some numbers with a little exercise: a cost benefit analysis (CBA) . Code reviews can save company’s money. I strongly suggest that you compile a table distinguishing between recurring and non-recurring costs for both development process with and without code reviews.

Fill up the table based on the average cost/hour of a team member.

For both recurring and non-recurring costs, add the various activities (e.g., issues due to bugs, time spent in troubleshooting, effort due to lack of clarity of documentation). Estimate the time spent for all the activities with and without code reviews/quality code (e.g., effort). Multiply the effort in hours and the average cost/hour in order to consider costs.

As a reminder, add the status in qualitative measure for bad code without reviews and qualitative benefits introduced by QA activities such as those explained in this book. Sum up all the costs for both approaches and you are done. You will end up quite impressed with the results.

Let’s consider a simplified scenario. Assume the average cost of development for a single engineer to be $100,000 per quarter (3 months). Also assume that you found a piece of technical debt that can be resolved by the team in 2 weeks. Let’s also assume that the team is composed of three engineers collaborating on the same codebase.

The beauty of this is that technical debt is a recurring cost. This means that not fixing it (in our example) will end up in costing $90,000 every quarter until it gets finally resolved.

Differently, not only fixing a given piece of technical debt has (in this case) a lower cost per quarter (i.e., $46,029). It saves money in the long run.

Indeed, suppose that the technical debt goes unfixed for an entire year. While fixing technical debt would stop at $46,029 in costs, the overall recurring cost for not fixing it would pile up to a grand total of $360,000 in a year. You can also quickly imagine how those numbers would snowball if we consider bigger teams and several pieces of technical debt that need to be resolved.