5.2 Code Quality Signals, Static Analysis, and Code Reviews

Code quality cannot be fully captured by a single score. Metrics like complexity, code duplication, test coverage, and alert counts serve as observation windows, they help teams identify sections worth reviewing. But none of them replace the need for judgment about business risks, design boundaries, and actual 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. It's important to maintain consistent measurements using the same tool over time, rather than manually calculating and comparing results with another tool.

EligibilityResult check(Registration r) {
    if (r.isBanned()) return REJECTED;
    if (r.level() < 18) return REJECTED;
    if (!r.hasPaid() && !r.hasWaiver()) return PENDING_PAYMENT;
    return ACCEPTED;
}

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
Circle/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	Fewer dependencies do not guarantee good design
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	High churn does not necessarily indicate poor design
Defect and Incident Records	Which modules have actually caused real losses	No incidents do not imply no latent risks

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.

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 feature 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 Executable Review Guidelines

Change descriptions must at least address:

## Why
Current issue, impact to users/The system approach, and why it’s being processed now.

## What
The boundaries of this change; nothing here has been altered.

## 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] Two retries will result in double deduction of quotas; make the command idempotent by registrationId.
[suggestion] This local variable could be replaced with a deadline to make timezone semantics clearer.
[question] When a refund fails after cancellation, which component restores the registration status?

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. Tools should enable teams to receive feedback, metrics are not the goal.

References

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