5.2 Code Quality Signals, Static Analysis, and Code Reviews

Code quality cannot be fully captured by a single score. Complexity, duplication, coverage, and warning counts help identify code worth reading. Assess that code against business risks, design boundaries, and runtime behavior.

Cyclomatic Complexity Measures Control Flow Paths

Cyclomatic Complexity is based on the number of independent paths in a control flow graph. In teaching contexts, it is often approximated as:

Complexity is approximately 1 + number of decision points

However, case, short-circuit boolean expressions, exception handling, and counting methods across different language constructs can vary depending on tool rules. Keep the tool version and rule options alongside each measurement so that trend comparisons use the same definition.

Save this complete Java 21 example as EligibilityRules.java and compile it with javac --release 21 EligibilityRules.java. Registration is a snapshot of the inputs; check requires a non-null argument. It keeps the inclusive level range [18, 100] used in 4.1.

public final class EligibilityRules {
    public enum Result { REJECTED, PENDING_PAYMENT, ACCEPTED }
    public record Registration(boolean banned, int level,
                               boolean paid, boolean waiver) {}

    public static Result check(Registration r) {
        if (r.banned()) return Result.REJECTED;
        if (r.level() < 18 || r.level() > 100) return Result.REJECTED;
        if (!r.paid() && !r.waiver()) return Result.PENDING_PAYMENT;
        return Result.ACCEPTED;
    }
}

Counting only the three if statements gives 4; counting the short-circuit || and && paths as well gives 6. PMD exposes an ignoreBooleanPaths option for this distinction. Cyclomatic complexity counts a basis of control-flow paths, not all possible execution histories; loops can produce arbitrarily many histories. A score of 6 does not imply that six tests prove the method correct. For example, banned=true must still reject a paid, in-range applicant, and waiver=true must allow an otherwise eligible unpaid applicant.

This function has multiple execution paths, but early returns and clear naming may still make it more readable than a version with lower complexity that hides logic within obscure expressions. When complexity increases, ask critical questions: are responsibilities mixed? Are domain concepts missing? Are test cases sufficient? Rather than mechanically splitting functions at a perceived threshold, examine the underlying design.

Combine Multiple Quality Signals

Signal	What it may indicate	What it cannot directly prove
Cyclomatic/Cognitive Complexity	Branching and mental load may be high	Defects are definitely present
Code Duplication	Changes might need to be synchronized across multiple locations	Two logical blocks must be abstracted into one
Dependencies and Coupling	Changes may propagate throughout the system	Good design from a low dependency count
Test Coverage	Which parts of the code have not been executed by tests	Assertions are sufficient and requirements are correctly defined
Change Frequency/Churn	Which areas are frequently touched	Poor design from high churn alone
Defect and Incident Records	Which modules have actually caused real losses	Absence of latent risks from a clean incident history

Combining signals like "high complexity + frequent changes + multiple incidents" typically provides more actionable insights than sorting the entire repository by a single, uniform threshold.

Static Analysis Is Repeatable, Automated Code Review

Analyzers outside the compiler can detect:

• Clear error patterns, such as null pointer dereferences or unclosed resources;
• Misuse of APIs and concurrency risks;
• Duplicate code, complexity, and unused code;
• Team-defined dependency and naming conventions;
• Certain security and data flow issues.

Rules should be categorized by risk level:

Blocking: High-confidence correctness, security, or compatibility issues
Warning: Design or maintainability concerns requiring human judgment
Information: Trend observations or gradual improvement initiatives

When suppressing warnings, clearly document the reason and scope. Globally disabling rules risks hiding real issues; a long-standing warning that goes unaddressed can inadvertently train the team to ignore CI feedback.

When upgrading analyzers or rule sets, lock down the version, review the change log, and address new warnings in a separate, isolated change, avoiding the mixing of tool updates with business logic changes.

A clean report means the enabled rules found no reportable issue within the configured analysis scope. It does not prove correctness: missing dependencies, reflection, generated code, and path approximations can cause missed findings or false alarms. A suppression should name the exact rule and code location, explain the relevant invariant, and keep a reproducing test where practical. “Legacy code” alone is not an explanation.

Code Review: Supplementing What Automation Can't See

Code reviews should prioritize confirming:

1. That requirements and designs address the right problems;
2. That design boundaries, data ownership, and failure semantics are reasonable;
3. That correctness, security, concurrency, and compatibility risks have been adequately addressed;
4. That tests cover the key risks introduced by this change;
5. That names, comments, and documentation explain why a decision was made, enabling future contributors to understand the rationale;
6. That complexity is proportionate to the current requirements.

Formatting, import ordering, and rules that can be automatically fixed should be left to tools. Human attention should be reserved for aspects that require context and judgment.

Small Changes Measured by One Concept

Change requests that are easy to review typically involve only one clearly independent, testable, and reversible action. Line count is just a reference: automatically generated files might be large but are easy to verify, while manually editing 200 lines across 50 files can be difficult to assess.

Larger features can be broken down into the following steps:

1. Add characterization tests
2. Pure renaming or moving
3. Introduce a new port while still using the old implementation
4. Add a new implementation and corresponding tests
5. Switch the call path
6. Remove the old implementation

Each step maintains system functionality, making it easier to detect errors and roll back changes than attempting to deliver all changes in a single, long-lived branch.

Write Review Descriptions That Can Be Checked

A change description should explain the problem, behavior, risk, and evidence. The following is a writing aid; a small change can cover these in a few sentences:

## Why
The current problem, its effect on users or the system, and why it needs attention.

## What
The behavior changed and the affected boundary.

## Risk
Failure modes, migration and compatibility impact, rollback strategy

## Verification
Automated testing, manual inspection, metrics, or screenshots

Reviews should distinguish between blocking issues and non-blocking suggestions, and clearly explain their impact:

[blocking] Retrying the same cancellation releases a place twice. Please make the cancellation transition idempotent for this registration and check that replay releases only one place in total.
[suggestion] Rename time to cancellationDeadline; the name will identify its role. Keep the time zone explicit in its type or conversion.
[question] If refunding fails after cancellation, does the registration stay CANCELLED, and which component retries the refund?

Review discussions should focus on code and constraints, not on the author. When disagreements arise, return to requirements, quality attributes, team standards, or experimental data. If a long-term rule emerges, document the conclusion in an ADR or engineering standard to avoid repeating debates in every PR.

How Metrics Can Return to Action

Production incidents and delivery resistance
  -> Identify high-risk code areas
    -> Incremental refactoring with additional testing
      -> Static rules and peer reviews prevent regressions
        -> Observe whether defect rates, delivery times, and recovery capabilities improve

If metrics improve without a reduction in defects, understanding time, or modification risk, revisit whether the metrics themselves are misaligned. A useful metric changes what the team investigates or repairs. A smaller number on its own is not enough.

References

• PMD: Java Rules
• Google Engineering Practices: What to Look for in a Code Review
• Google Engineering Practices: Small CLs