code-quality-review

name	code-quality-review
description	Systematic code review patterns, quality dimensions, anti-pattern detection, and constructive feedback techniques. Use when reviewing code changes, assessing codebase quality, identifying technical debt, or mentoring through reviews. Covers correctness, design, security, performance, and maintainability.

Code Quality Review Methodology

Systematic patterns for reviewing code and providing constructive, actionable feedback that improves both code quality and developer skills.

When to Activate

• Reviewing pull requests or merge requests
• Assessing overall codebase quality
• Identifying and prioritizing technical debt
• Mentoring developers through code review
• Establishing code review standards for teams
• Auditing code for security or compliance

Review Dimensions

Every code review should evaluate these six dimensions:

1. Correctness

Does the code work as intended?

Check	Questions
Functionality	Does it solve the stated problem?
Edge Cases	Are boundary conditions handled?
Error Handling	Are failures gracefully managed?
Data Validation	Are inputs validated at boundaries?
Null Safety	Are null/undefined cases covered?

2. Design

Is the code well-structured?

Check	Questions
Single Responsibility	Does each function/class do one thing?
Abstraction Level	Is complexity hidden appropriately?
Coupling	Are dependencies minimized?
Cohesion	Do related things stay together?
Extensibility	Can it be modified without major changes?

3. Readability

Can others understand this code?

Check	Questions
Naming	Do names reveal intent?
Comments	Is the "why" explained, not the "what"?
Formatting	Is style consistent?
Complexity	Is cyclomatic complexity reasonable (<10)?
Flow	Is control flow straightforward?

4. Security

Is the code secure?

Check	Questions
Input Validation	Are all inputs sanitized?
Authentication	Are auth checks present where needed?
Authorization	Are permissions verified?
Data Exposure	Is sensitive data protected?
Dependencies	Are there known vulnerabilities?

5. Performance

Is the code efficient?

Check	Questions
Algorithmic	Is time complexity appropriate?
Memory	Are allocations reasonable?
I/O	Are database/network calls optimized?
Caching	Is caching used where beneficial?
Concurrency	Are race conditions avoided?

6. Testability

Can this code be tested?

Check	Questions
Test Coverage	Are critical paths tested?
Test Quality	Do tests verify behavior, not implementation?
Mocking	Are external dependencies mockable?
Determinism	Are tests reliable and repeatable?
Edge Cases	Are boundary conditions tested?

Anti-Pattern Catalog

Common code smells and their remediation:

Method-Level Anti-Patterns

Anti-Pattern	Detection Signs	Remediation
Long Method	>20 lines, multiple responsibilities	Extract Method
Long Parameter List	>3-4 parameters	Introduce Parameter Object
Duplicate Code	Copy-paste patterns	Extract Method, Template Method
Complex Conditionals	Nested if/else, switch statements	Decompose Conditional, Strategy Pattern
Magic Numbers	Hardcoded values without context	Extract Constant
Dead Code	Unreachable or unused code	Delete it

Class-Level Anti-Patterns

Anti-Pattern	Detection Signs	Remediation
God Object	>500 lines, many responsibilities	Extract Class
Data Class	Only getters/setters, no behavior	Move behavior to class
Feature Envy	Method uses another class's data extensively	Move Method
Inappropriate Intimacy	Classes know too much about each other	Move Method, Extract Class
Refused Bequest	Subclass doesn't use inherited behavior	Replace Inheritance with Delegation
Lazy Class	Does too little to justify existence	Inline Class

Architecture-Level Anti-Patterns

Anti-Pattern	Detection Signs	Remediation
Circular Dependencies	A depends on B depends on A	Dependency Inversion
Shotgun Surgery	One change requires many file edits	Move Method, Extract Class
Leaky Abstraction	Implementation details exposed	Encapsulate
Premature Optimization	Complex code for unproven performance	Simplify, measure first
Over-Engineering	Abstractions for hypothetical requirements	YAGNI - simplify

Review Prioritization

Focus review effort where it matters most:

Priority 1: Critical (Must Fix)

• Security vulnerabilities (injection, auth bypass)
• Data loss or corruption risks
• Breaking changes to public APIs
• Production stability risks

Priority 2: High (Should Fix)

• Logic errors affecting functionality
• Performance issues in hot paths
• Missing error handling for likely failures
• Violation of architectural principles

Priority 3: Medium (Consider Fixing)

• Code duplication
• Missing tests for new code
• Naming that reduces clarity
• Overly complex conditionals

Priority 4: Low (Nice to Have)

• Style inconsistencies
• Minor optimization opportunities
• Documentation improvements
• Refactoring suggestions

Constructive Feedback Patterns

The Feedback Formula

[Observation] + [Why it matters] + [Suggestion] + [Example if helpful]

Good Feedback Examples

# Instead of:
"This is wrong"

# Say:
"This query runs inside a loop (line 45), which could cause N+1
performance issues as the dataset grows. Consider using a batch
query before the loop:

```python
users = User.query.filter(User.id.in_(user_ids)).all()
user_map = {u.id: u for u in users}

"

```markdown
# Instead of:
"Use better names"

# Say:
"The variable `d` on line 23 would be clearer as `daysSinceLastLogin` -
it helps readers understand the business logic without tracing back
to the assignment."

Feedback Tone Guide

Avoid	Prefer
"You should..."	"Consider..." or "What about..."
"This is wrong"	"This might cause issues because..."
"Why didn't you..."	"Have you considered..."
"Obviously..."	"One approach is..."
"Always/Never do X"	"In this context, X would help because..."

Positive Observations

Include what's done well:

"Nice use of the Strategy pattern here - it makes adding new
payment methods straightforward."

"Good error handling - the retry logic with exponential backoff
is exactly what we need for this flaky API."

"Clean separation of concerns between the validation and persistence logic."

Review Checklists

Quick Review Checklist (< 100 lines)

• Code compiles and tests pass
• Logic appears correct for stated purpose
• No obvious security issues
• Naming is clear
• No magic numbers or strings

Standard Review Checklist (100-500 lines)

All of the above, plus:

• Design follows project patterns
• Error handling is appropriate
• Tests cover new functionality
• No significant duplication
• Performance is reasonable

Deep Review Checklist (> 500 lines or critical)

All of the above, plus:

• Architecture aligns with system design
• Security implications considered
• Backward compatibility maintained
• Documentation updated
• Migration/rollback plan if needed

Review Workflow

Before Reviewing

1. Understand the context (ticket, discussion, requirements)
2. Check if CI passes (don't review failing code)
3. Estimate review complexity and allocate time

During Review

1. First pass: Understand the overall change
2. Second pass: Check correctness and design
3. Third pass: Look for edge cases and security
4. Document findings as you go

After Review

1. Summarize overall impression
2. Clearly indicate approval status
3. Distinguish blocking vs non-blocking feedback
4. Offer to discuss complex suggestions

Review Metrics

Track review effectiveness:

Metric	Target	What It Indicates
Review Turnaround	< 24 hours	Team velocity
Comments per Review	3-10	Engagement level
Defects Found	Decreasing trend	Quality improvement
Review Time	< 60 min for typical PR	Right-sized changes
Approval Rate	70-90% first submission	Clear standards

Anti-Patterns in Reviewing

Avoid these review behaviors:

Anti-Pattern	Description	Better Approach
Nitpicking	Focusing on style over substance	Use linters for style
Drive-by Review	Quick approval without depth	Allocate proper time
Gatekeeping	Blocking for personal preferences	Focus on objective criteria
Ghost Review	Approval without comments	Add at least one observation
Review Bombing	Overwhelming with comments	Prioritize and limit to top issues
Delayed Review	Letting PRs sit for days	Commit to turnaround time

References

• Review Dimension Details - Expanded criteria for each dimension
• Anti-Pattern Examples - Code examples of each anti-pattern