Code Review Skill

When to Use

Invoke this skill when reviewing code for architectural soundness, design patterns, SOLID compliance, and software engineering best practices.

Review Checklist

SOLID Principles

Core Software Engineering Principles

Top 10 Code Smells

1. Memory leaks - Resources not released, will lead to crashes
2. Improperly disposed objects - Missing cleanup, resource exhaustion
3. ORM misuse - N+1 queries, lazy loading issues, missing indexes
4. Security flaws - Injection, auth bypass, data exposure
5. Thread-safety issues - Race conditions, deadlocks, shared mutable state
6. Object lifetime problems - Premature disposal, zombie objects
7. SOLID violations - See checklist above
8. Poor exception handling - Swallowed exceptions, missing context
9. Scalability blockers - Synchronous bottlenecks, unbounded resources
10. Cache/session misuse - Stale data, memory bloat, missing invalidation

Architecture Styles

Three-Tier Architecture

• Presentation - UI/API layer
• Business - Domain logic
• Data - Persistence layer
• All tiers are physically separate (unlike MVC which is logical separation)

Monolith

Characteristics:

• Single codebase, unified deployment
• Tight coupling between components
• Centralized data management

Advantages:

• Simplicity for small/moderate complexity
• Performance (no inter-service communication)
• Easier deployment

Disadvantages:

• Maintenance challenges as it grows
• Scalability limitations (can't scale parts independently)
• Technology lock-in
• Bug in one subsystem can crash entire application

Service-Oriented Architecture (SOA)

Key Principles:

• Well-defined, self-contained services
• Standardized communication protocols (XML, JSON)
• Loose coupling between services
• Service discoverability via registry
• Reusable, composable components

Microservices

Characteristics:

• Small, independently deployable services
• Single responsibility per service
• Loosely coupled via APIs (HTTP, gRPC, message queues)
• Technology agnostic
• Decentralized data management
• Failure isolation

Review Questions:

• Is this service aligned with a bounded context?
• Can it be deployed independently?
• Does it communicate via messaging?
• Is it autonomously developed?

Design Patterns

Creational Patterns

Focus: Object instantiation

Structural Patterns

Focus: Composition, readability, maintainability

Behavioral Patterns

Focus: Object communication, modifiability

Clean Architecture

Goal: Make the business layer immune from changes elsewhere in the application.

Hexagonal Architecture (Ports & Adapters)

• Separation via ports (interfaces) and adapters (implementations)
• External dependencies easily mocked
• Business logic at the center

Onion Architecture

• Organized in concentric circles
• Inner layers CAN depend on outer layers
• Outer layers CANNOT depend on inner layers
• Core domain at the center

Domain-Driven Design Concepts

Bounded Context

A logical boundary within which a specific domain model is defined and consistently used.

Purpose:

• Manage complexity by dividing into smaller pieces
• Ensure consistency within the boundary
• Isolate changes from other contexts

Review Questions:

• Are terms/entities used consistently within this context?
• Are integration points with other contexts well-defined?
• Does this context have a clear responsibility?

Fit For Purpose

Software is "fit for purpose" when it meets:

Event-Driven Architecture Patterns

Anti-Patterns to Flag

Cargo Cult Programming

Dogmatic, ritual, blind application of patterns without understanding purpose.

Signs:

• Pattern used but provides no benefit
• Complexity added without solving a problem
• "We always do it this way" reasoning

Reminder: Every pattern should serve a real purpose for the specific asset.

Quick Reference