| name | review-changes-java |
| description | Systematic code review workflow to evaluate changes against Java and Spring standards. Use when reviewing Pull Requests, commits, or diffs. Ensures robustness, maintainability, and adherence to best practices. |
Review Changes (Java/Spring Boot)
Systematic workflow for reviewing Java/Spring code against established standards.
1. Understand the Change
- What problem does this solve?
- What is the scope of impact?
- Are tests included and do they cover the use cases?
2. Java Best Practices Review
- Robustness:
- Is
try-with-resourcesused to close resources? - Is exception handling correct?
- Is
- Immutability:
- Are fields declared
finalwhere possible? - Could the class be immutable?
- Are fields declared
- Types:
- Are Raw Types (e.g.,
List) avoided? - Is
Optionalused correctly (e.g.,isPresent()followed byget(), or preferorElse,map)?
- Are Raw Types (e.g.,
- Contracts:
- If
equals()is implemented, ishashCode()also implemented?
- If
- Lambdas/Streams:
- Is the streams code readable?
- Are Method References used where possible?
- Naming:
- Clear and descriptive names (no abbreviations)?
3. Spring Patterns Review
- Dependency Injection:
- Is injection done via constructor? (Avoid
@Autowiredon fields).
- Is injection done via constructor? (Avoid
- Configuration:
- Is it using
@ConfigurationProperties(preferred) or@Value?
- Is it using
- Layers:
- Is business logic in the
@Service? - Is the
@Controlleronly orchestrating the call (no business logic)? - Is the
@Repositorycalled directly by theController? (Should not be).
- Is business logic in the
- Transactions:
- Is
@Transactionalused correctly (generally on publicServicemethods)?
- Is
- Web:
- Is
WebClientused instead ofRestTemplate?
- Is
- Security:
- Are endpoints secured by Spring Security?
- Is sensitive data excluded from logs?
4. Testing
- Unit Tests: Use Mockito correctly?
- Test Slices: Is the correct slice used (e.g.,
@WebMvcTest)? - Testcontainers: Is it used for integration tests with a real database?
- Assertions: Are assertions clear and testing behavior, not implementation?
5. Feedback Format
Required Changes (Blocking)
- Security vulnerabilities.
- Violations of Java contracts (e.g., equals/hashCode).
- Use of Raw Types.
- Field-based dependency injection.
Suggested Improvements (Non-blocking)
- Refactoring opportunities (e.g., use Method Reference).
- Naming could be clearer.
- Convert
@Valueto@ConfigurationProperties.
Positive Feedback
- Great use of Testcontainers!
- Good application of the Builder pattern.