Code Review Checklist and Best Practices in Java

Standard Operating Procedure: Code Review Checklist and Best Practices in Java

1. Document Control & Revision History

---

2. Executive Summary & Scope

2.1 Clinical Statement of Operational Boundaries

This Standard Operating Procedure (SOP) defines the mandatory engineering controls, inspection protocols, and review criteria governing all Java source code changes within enterprise production systems. The lifecycle procedures outlined herein are engineered to eliminate architectural drift, enforce defensive programming paradigms, mitigate systemic security vulnerabilities (OWASP Top 10), and maintain absolute compliance with institutional SLAs, ISO/IEC 25010 maintainability benchmarks, and SOC2/HIPAA data integrity mandates.

2.2 Operational Scope

This document applies universally to:

• All software engineers, senior developers, and technical leads contributing to Java-based microservices, monolithic applications, libraries, and data-processing pipelines.
• Code artifacts targeting Java Platform, Standard Edition (Java SE) versions 17 LTS through 21 LTS.
• Integrations utilizing Spring Framework, Quarkus, Jakarta EE, Hibernate ORM, and associated enterprise runtimes.

---

3. Prerequisites & Tool Requirements

Execution of this SOP requires mandatory verification of the following hardware, software environments, and authorization profiles:

• Integrated Development Environment (IDE) / Tooling: IntelliJ IDEA Ultimate Edition (v2023.x+) or Eclipse IDE with mandatory plugins: SonarLint, Checkstyle-IDEA, and SpotBugs.
• Build & Dependency Systems: Apache Maven (v3.8.x+) or Gradle (v8.x+), incorporating automated static analysis execution via the build pipeline.
• Version Control System (VCS): Enterprise GitHub Enterprise, GitLab Ultimate, or Bitbucket Data Center with branch protection rules enforced (requiring linear history, status checks, and mandatory peer approvals).
• Access Control & Certifications:
  • Authorized membership in the eng-code-reviewers LDAP/Active Directory security group.
  • Passing grade (>= 90%) on the Enterprise Secure Coding in Java internal certification assessment.
• Required CI/CD Quality Gates: Automated execution pipelines generating reports for JaCoCo (code coverage), SonarQube (security/maintainability metrics), and OWASP Dependency-Check.

---

4. Roles & Responsibilities (RACI Matrix Table)

• Responsible (R): The role that performs the activity or review step.
• Accountable (A): The role with final approval authority and ultimate liability.
• Consulted (C): The role providing subject-matter expertise, security clearances, or architectural alignment.
• Informed (I): The role kept updated on review status and merge deployment.

---

5. Step-by-Step Operational Procedure

Phase 1: Pre-Review Triage and Automated Gate Verification

Note: The reviewer must not initiate manual line-by-line inspection until all automated baseline checks return a passing status.

• Verify that the pull request (PR) references an approved Jira ticket, issue identifier, or engineering change request (ECR).
• Confirm that all automated CI/CD pipeline checks (Build, Unit Tests, Integration Tests) have passed successfully with 0 compilation warnings.
• Validate that JaCoCo code coverage metrics meet or exceed the enterprise threshold of 85% instruction coverage and 80% branch coverage for modified packages.
• Inspect SonarQube scan results to ensure 0 Blocker, 0 Critical, and 0 Major bugs or security hotspots exist in the diff.
• Ensure the PR branch is rebased cleanly onto the target branch (typically main or release branches) with no unresolved merge conflicts.

Phase 2: Architectural and Design Pattern Inspection

• Verify adherence to Domain-Driven Design (DDD) boundaries; ensure domain models do not leak persistence logic or infrastructure annotations (e.g., Hibernate @Entity inside core domain services).
• Validate that dependency injection via constructor wiring is utilized uniformly; reject field injection (@Autowired on fields) due to immutability and testability failures.
• Confirm that interface-based programming is applied to service boundaries (e.g., PaymentService implemented by StripePaymentServiceImpl) to support mocking and proxying.
• Check that thread safety requirements are rigorously maintained: mutable shared state must be protected via synchronized blocks, atomic variables, or concurrent collections (java.util.concurrent).
• Validate proper utilization of modern Java features (records for immutable data transfer objects, pattern matching for switch, and sealed classes for exhaustive type hierarchies).

Phase 3: Defensive Programming, Error Handling, and Resource Management

• Verify that exception handling is granular; ensure raw Exception, Throwable, or RuntimeException are never caught generically unless wrapped in a global exception handler.
• Confirm that all checked and custom business exceptions inherit from a standardized base application exception and contain contextual error codes.
• Check that try-with-resources statements are employed for all classes implementing java.lang.AutoCloseable (e.g., JDBC Connection, Statement, ResultSet, file I/O streams) to prevent resource leaks.
• Verify that input validation is executed at system ingress points (REST controllers, message consumers) utilizing Jakarta Validation API (@NotNull, @Size, @Email, custom validators).
• Inspect null-safety handling: ensure strategic usage of java.util.Optional for return types that may legally yield absent values, avoiding the unchecked return of null.

Phase 4: Performance, Concurrency, and Memory Management

• Review collections usage for proper sizing initialization (e.g., new ArrayList<>(expectedSize)) to eliminate costly dynamic array resizing operations under high throughput.
• Verify that string concatenation inside loops uses StringBuilder or StringBuffer to prevent excessive object allocation and garbage collection (GC) pressure.
• Validate database query performance: inspect JPA/Hibernate repositories for the prevention of N+1 select problems (ensure @EntityGraph, JOIN FETCH, or batch fetching is utilized for associations).
• Ensure that transactional boundaries (@Transactional) are applied strictly at the service layer, optimizing isolation levels (defaulting to READ_COMMITTED) and designating read-only transactions where applicable (readOnly = true).
• Verify that asynchronous operations (CompletableFuture, ExecutorService) define bounded thread pools with appropriate rejection policies, explicitly avoiding unchecked default ForkJoinPool saturations.

Phase 5: Security, Data Protection, and Compliance

• Confirm complete mitigation of SQL/JPQL injection: ensure all queries utilize parameterized statements, criteria builders, or safe ORM repository abstractions; dynamic query concatenation is strictly prohibited.
• Verify protection against Cross-Site Scripting (XSS) and Injection vectors by ensuring all unrusted output is properly sanitized and encoded.
• Inspect cryptographic operations: ensure approved cipher suites and hashing algorithms are utilized (e.g., AES-GCM-256 for symmetric encryption, Argon2 or BCrypt for password hashing); custom cryptography implementations are banned.
• Validate that sensitive data (PII, PCI-DSS parameters, tokens, credentials) are explicitly scrubbed, masked, or omitted from standard logging frameworks (SLF4J/Logback).
• Verify that dependency updates do not introduce known Vulnerabilities (CVEs) as validated by OWASP Dependency-Check or Snyk reports.

Phase 6: Code Readability, Style, and Test Quality

• Ensure adherence to the Google Java Style Guide or institutional Checkstyle profile (indentation, naming conventions, max line length limits).
• Confirm that all public classes, methods, and interfaces feature comprehensive Javadoc documentation explaining operational contracts, preconditions, and postconditions.
• Verify that dead code, commented-out blocks, legacy debugging statements (System.out.println), and unused imports are entirely removed from the diff.
• Review unit and integration tests (JUnit 5, Mockito, AssertJ) for operational validity: tests must assert deterministic behavior, test boundary conditions, and mock external network boundaries correctly.

---

6. Quality Assurance & Verification Criteria

To secure final merge authorization, the code artifact must satisfy the following explicit quantitative and qualitative benchmarks:

---

7. Practical Troubleshooting Scenarios

Scenario 1: The N+1 Database Query Performance Bottleneck

• Symptom: During Phase 4 inspection, a JPA entity relationship (@OneToMany or @ManyToMany) is fetched inside a loop, triggering hundreds of individual SQL statements during integration test execution, resulting in severe latency spikes.
• Immediate Corrective Action Protocol:
  1. Halt the review approval process and comment directly on the offending repository/service line using the tag [BLOCKER-PERF].
  2. Require the developer to refactor the query to use a JPQL fetch join (SELECT DISTINCT o FROM Order o JOIN FETCH o.lineItems WHERE o.id = :id) or apply the @EntityGraph annotation to the repository method.
  3. Mandate the addition of a query count assertion test (utilizing frameworks like Hibernate-Inspector or P6Spy) to ensure the operation executes in exactly one SQL round-trip.

Scenario 2: Uncaught Runtime Exception Leakage & Information Disclosure

• Symptom: A custom REST controller catches Exception and returns the raw e.getMessage() or full stack trace directly to the HTTP client response body in the event of a persistence or external API failure.
• Immediate Corrective Action Protocol:
  1. Flag the vulnerability immediately using the tag [BLOCKER-SEC].
  2. Direct the developer to remove direct exception exposure from the controller layer.
  3. Enforce the implementation of a centralized global exception handler annotated with @ControllerAdvice and @ExceptionHandler.
  4. Ensure internal system errors are mapped to generic, sanitized HTTP status codes (e.g., 500 Internal Server Error with an opaque tracking UUID) while full stack traces are securely written exclusively to structured JSON logs via SLF4J.

Scenario 3: Potential Memory Leak via Static Collection Reference

• Symptom: A caching mechanism or listener registration appends objects to a static HashMap or List within a singleton service bean without a corresponding eviction policy, TTL, or explicit removal method.
• Immediate Corrective Action Protocol:
  1. Flag the memory retention vulnerability using [BLOCKER-MEM].
  2. Require the removal of static collection references holding domain state across application lifecycles.
  3. Mandate the integration of standard caching abstractions, specifically Spring Cache coupled with a bounded backend (e.g., Caffeine cache featuring .maximumSize() and .expireAfterWrite() configurations) to guarantee automatic garbage collection of stale references.