Code Quality

Code quality encompasses readability, maintainability, correctness, and performance. High-quality code is easy to understand, modify, and extend.

Code Reviews

Purpose

Find bugs before they reach production.
Share knowledge across the team.
Enforce standards (style, architecture, patterns).
Improve design through discussion.

Best Practices

Author: Keep PRs small (< 400 lines). Write a clear description. Self-review before requesting review. Link to issues/specs.

Reviewer: Be constructive, not critical of the person. Focus on correctness, design, and readability. Distinguish between blocking issues and suggestions. Review promptly (< 24 hours).

What to look for: Logic errors, edge cases, security issues, performance concerns, test coverage, naming/readability, code duplication, adherence to patterns.

Static Analysis and Linting

Linters

Enforce code style and catch common mistakes.

# Rust
cargo clippy         # lint: suggest idiomatic improvements
cargo fmt --check    # format: check code style consistency

# Examples of Clippy suggestions:
# - Use `is_empty()` instead of `len() == 0`
# - Use `if let` instead of `match` with one arm
# - Avoid `clone()` when borrow would work

Static Analysis Tools

Type checking: Rust's compiler catches many bugs at compile time (null safety, ownership, thread safety).

Beyond type checking: Semgrep (custom rules), SonarQube (quality gates), CodeClimate.

Formal-ish analysis: Miri (Rust: detect undefined behavior in unsafe code), cargo-careful (extra runtime checks).

Code Smells

Indicators that code may need refactoring (Fowler & Beck):

| Smell | Description | Refactoring | |---|---|---| | Long method | Method too large to understand | Extract method | | Large class | Class with too many responsibilities | Extract class, SRP | | Duplicate code | Same logic in multiple places | Extract method/module | | Feature envy | Method uses another class's data | Move method | | Data clump | Same group of data appears together | Extract struct | | Primitive obsession | Use primitives instead of domain types | Introduce value objects | | Switch statements | Complex conditionals on types | Replace with polymorphism | | Long parameter list | Too many function parameters | Introduce parameter object, builder | | Speculative generality | Unused abstractions "for the future" | Remove (YAGNI) | | Dead code | Unreachable or unused code | Delete it |

Refactoring Techniques

Extract Method

Move a code block into a named method.

// Before
PROCEDURE PROCESS_ORDER(order)
    // validate
    IF order.items is empty THEN ERROR "empty order"
    IF order.TOTAL() < 0 THEN ERROR "negative total"
    // save
    db.SAVE(order)
    // notify
    email.SEND(order.USER_EMAIL(), "Order confirmed")

// After
PROCEDURE PROCESS_ORDER(order)
    VALIDATE_ORDER(order)
    SAVE_ORDER(order)
    NOTIFY_CUSTOMER(order)

Rename

Give variables, functions, and types descriptive names.

// Before
x ← GET_DATA(42)
r ← CALC(x, 0.15)

// After
order ← FETCH_ORDER(42)
discounted_total ← APPLY_DISCOUNT(order, 0.15)

Replace Conditional with Polymorphism

// Before
FUNCTION CALCULATE_AREA(shape)
    IF shape.kind = "circle" THEN RETURN PI * shape.radius^2
    IF shape.kind = "rectangle" THEN RETURN shape.width * shape.height
    ELSE ERROR "unknown shape"

// After
INTERFACE Shape   FUNCTION AREA() -> number
CLASS Circle      FUNCTION AREA()  RETURN PI * self.radius^2
CLASS Rectangle   FUNCTION AREA()  RETURN self.width * self.height

Technical Debt

Metaphor (Cunningham): Shipping imperfect code is like taking on debt. You pay "interest" (slower development, more bugs) until you "repay" (refactor).

Types:

Deliberate: Conscious shortcuts ("We know this won't scale, but we need to ship. We'll fix it in Q2.")
Inadvertent: Accidental complexity from lack of knowledge or evolving requirements.
Bit rot: Working code degrades over time as the environment changes.

Managing debt: Track in issue tracker. Allocate time for repayment (e.g., 20% of sprint capacity). Prioritize by impact. Don't let it accumulate indefinitely.

Documentation

Types

API documentation: Function/method docs. In Rust: /// doc comments → auto-generated with cargo doc.

// Calculates the factorial of a non-negative integer.
//
// Arguments:
//   n - A non-negative integer
//
// Returns:
//   The factorial of n, or NONE if the result would overflow.
//
// Examples:
//   FACTORIAL(5) = Some(120)
//   FACTORIAL(0) = Some(1)

PUBLIC FUNCTION FACTORIAL(n)   // ...

Architecture Decision Records (ADRs): Document why decisions were made (covered in architectural patterns).

Runbooks: Step-by-step guides for operational procedures (deploy, rollback, debug, incident response).

README: Project overview, setup instructions, usage examples. The first thing a new developer reads.

When to Write Comments

Don't comment WHAT (the code already says that):

// BAD: increment counter
counter ← counter + 1

Comment WHY (the code doesn't explain intent):

// Use a buffer size of 8192 because benchmarks showed this minimizes
// syscall overhead while fitting in L1 cache.
CONST BUFFER_SIZE ← 8192

Comment non-obvious behavior:

// SAFETY: We know the slice is valid because we just allocated it
// and the pointer hasn't been shared yet.
UNSAFE CREATE_SLICE(ptr, len)

Metrics

Cyclomatic Complexity

Count of independent paths through the code. if adds 1, each match arm adds 1, loops add 1.

Complexity = Edges - Nodes + 2 (in the control flow graph)

Guidelines: < 10 is good. 10-20 is acceptable. > 20 needs refactoring.

Cognitive Complexity (SonarQube)

Measures how hard the code is to understand (not just the number of paths). Nesting increases complexity. break/continue/goto add complexity.

Coupling and Cohesion

Afferent coupling (Ca): How many other modules depend on this one. High = many dependents = change is risky.

Efferent coupling (Ce): How many modules this one depends on. High = many dependencies = fragile.

Instability: I = Ce / (Ca + Ce). Ranges from 0 (stable) to 1 (unstable).

Applications in CS

Team productivity: Good code quality reduces time spent debugging and understanding code. 10× difference between well-maintained and poorly-maintained codebases.
Onboarding: Clean code and good documentation dramatically reduce onboarding time.
Reliability: Code reviews and static analysis catch bugs before production.
Maintenance: Refactoring keeps code flexible. Technical debt management prevents slowdowns.
Open source: Code quality determines whether contributors can understand and contribute to a project.