Module 5: Applied Design and Code Review: Case Studies
These cases turn design judgment into reviewable artifacts: review comments, ADRs, migration plans, and test evidence.
Case Study 1: The Refactor PR Nobody Can Review
Scenario: A developer opens a 4,000-line pull request that renames classes, moves files, changes payment behavior, updates tests, and adds a new retry policy. Reviewers can see the final shape but cannot isolate risk.
Source anchor: Google Engineering Practices: Small CLs.
Module concepts:
- small changes
- reviewability
- behavior-preserving refactor
- risk isolation
Wrong Approach
Ask reviewers to "review by commit" while the commits still mix mechanical moves and behavior changes.
Better Approach
Split the work into reviewable changes: mechanical rename, behavior-preserving extraction, adapter introduction, retry behavior, and cleanup. Each change should pass tests independently and have a clear rollback story.
Tradeoff Table
| Choice | Gain | Cost |
|---|---|---|
| Huge PR | One visible endpoint | Review fatigue and hidden regressions |
| Small CL sequence | Focused review | More coordination |
| Feature flag path | Safer rollout | Extra temporary code |
Failure Mode
Reviewers approve the design but miss that retry behavior now double-charges one payment path.
Required Artifact
Create a PR split plan with sequence, purpose, expected diff type, tests, and rollback for each step.
Project / Capstone Connection
Use this split plan for the final Semester 3 refactoring submission.
Case Study 2: Review Comment That Finds the Design Bug
Scenario: A pull request adds if user.role == "admin" checks in five controllers. Tests pass, but the design spreads authorization policy across request handlers.
Source anchor: Google Engineering Practices: Code Review.
Module concepts:
- code review as design feedback
- policy centralization
- coupling
- actionable review comments
Wrong Approach
Leave vague comments such as "this feels messy" or block the PR without proposing a path.
Better Approach
Write a specific design comment: name the risk, point to duplicated policy, propose an authorization boundary, and request tests at the boundary. Separate must-fix safety issues from optional cleanup.
Tradeoff Table
| Choice | Gain | Cost |
|---|---|---|
| Vague review | Fast to write | Hard for author to act on |
| Specific boundary request | Improves design | Requires reviewer effort |
| Full redesign demand | May produce best architecture | Can block urgent value |
Failure Mode
One controller forgets the admin check and exposes an internal operation.
Required Artifact
Write three review comments: one blocking, one suggestion, and one question. Each must include risk, evidence, and requested action.
Project / Capstone Connection
Use this comment format during peer review of project submissions.
Case Study 3: ADR Before Introducing a Pattern
Scenario: A team wants to add a message bus and observers for every domain event. The current system has only two consumers and no clear delivery requirements.
Source anchor: Design Patterns Catalog - Refactoring.Guru and the module's applied design review concepts.
Module concepts:
- ADRs
- pattern fit
- overengineering
- decision consequences
Wrong Approach
Introduce the message bus because it is a known pattern and might be useful later.
Better Approach
Write an ADR that states the problem, forces, options, decision, consequences, and revisit trigger. Compare direct calls, in-process events, queue-backed events, and no change against actual requirements.
Tradeoff Table
| Choice | Gain | Cost |
|---|---|---|
| Direct calls | Simple and observable | Tighter coupling |
| In-process events | Extension point | Hidden ordering concerns |
| Message bus | Strong decoupling | Operational burden |
Failure Mode
The team adds asynchronous behavior without deciding retry, ordering, or failure semantics.
Required Artifact
Write a one-page ADR with options, decision, consequences, and the metric or requirement that would trigger reconsideration.
Project / Capstone Connection
Use this ADR format before adding any pattern to the final design.
Case Study 4: Tests That Prove a Refactor Stayed Honest
Scenario: A notification module is refactored from procedural code to strategies and adapters. Existing unit tests check private helper methods, so most tests break even when behavior is correct.
Source anchor: Refactoring catalog - Martin Fowler.
Module concepts:
- behavior-focused tests
- characterization tests
- public contract
- refactoring safety
Wrong Approach
Rewrite tests to match every new class and treat that as proof that behavior stayed the same.
Better Approach
Define the observable contract first: messages sent, retries attempted, invalid recipients rejected, and audit records written. Keep or add tests at that boundary, then refactor internals freely.
Tradeoff Table
| Choice | Gain | Cost |
|---|---|---|
| Private-method tests | Precise local feedback | Brittle under refactor |
| Contract tests | Protect behavior | May be slower |
| Snapshot-only tests | Easy to generate | Can hide missing assertions |
Failure Mode
All rewritten unit tests pass, but retry behavior changed because no boundary test captured it.
Required Artifact
Create a test coverage map with behavior, test level, fixture, assertion, and refactor risk covered.
Project / Capstone Connection
Attach the coverage map to the project refactoring report.
Case Study 5: Debt Ticket That Engineers Can Actually Use
Scenario: After a sprint, the team logs a ticket called "clean up design." It sits untouched because nobody knows what code, risk, or outcome it refers to.
Source anchor: Google Engineering Practices: Code Review, applied to actionable engineering feedback.
Module concepts:
- technical debt framing
- actionable scope
- evidence-based prioritization
- review follow-through
Wrong Approach
Create a broad debt epic with no failing examples, owner, or acceptance criteria.
Better Approach
Write a focused debt ticket: name the smell, show evidence, explain user or engineering risk, set non-goals, define acceptance criteria, and link the tests or metrics that prove improvement.
Tradeoff Table
| Choice | Gain | Cost |
|---|---|---|
| Broad cleanup ticket | Captures frustration | Not actionable |
| Focused debt ticket | Can be scheduled | Requires evidence |
| Ignore debt | Keeps sprint moving | Design cost compounds |
Failure Mode
The same review comment repeats for months because no one converted it into executable work.
Required Artifact
Write a debt ticket with title, evidence, impact, proposed change, non-goals, acceptance criteria, and review owner.
Project / Capstone Connection
Include one focused debt ticket in the final project portfolio.
Source Map
| Source | Use it for |
|---|---|
| Google Engineering Practices: Small CLs | Splitting design work into reviewable changes. |
| Google Engineering Practices: Code Review | Review expectations and actionable design feedback. |
| Refactoring catalog - Martin Fowler | Anchoring refactoring safety and behavior-preserving change. |
| Design Patterns Catalog - Refactoring.Guru | Evaluating whether pattern use is justified by design pressure. |