Skip to content
Ben Peetermans

Ben Peetermans

Builder. 20 years shipping for the web.

Let's talk
Build Notes
Build Notes

5 Structural Patterns That Survive Every Code Review

File-by-file review reads each file in isolation and pronounces it fine. These 5 cross-file patterns are invisible from inside any single file — and they compound on every change.

5 Structural Patterns That Survive Every Code Review

Code review reads files one at a time. Each file looks fine. The review passes.

But some patterns only exist between files. Duplicate methods across three controllers. A model method that nobody calls because every controller reinvented it. A 438-line controller that only looks fat when you see its 168-line peers doing the same job.

These patterns don’t break anything. They compound. Every copy-paste spreads them. Every new developer copies the pattern they see first. The codebase “works,” and gets harder to change. This is exactly how technical debt accumulates in a codebase without ever triggering a visible error.

I ran a structural graph scan (using LayerView connected to Claude Code via MCP) on a Laravel project before launch. (The same approach I used to narrow 733 dead code candidates down to 9 confirmed deletes.) 122 files, 14 controllers, 68 routes. Everything worked. The graph found 5 patterns that manual review never would have.

1. The helper method that lives in three places

Three controllers each had a private getProperty() method. Same 8 lines. Same logic: check the request for a property param, fall back to the active property.

Each controller reads fine on its own. You’d never think “I should search the codebase for other copies of this method” while reviewing a controller that works.

Why review misses it: You read AppController on Monday. DigestController on Wednesday. AlertController next week. Each one looks correct. The duplication is invisible because you never see all three at once.

How to check:

# Find methods defined in multiple controllers
grep -rn "private function\|protected function" app/Http/Controllers/ \
  | awk -F'function ' '{print $2}' | sort | uniq -c | sort -rn | head -10

If the same method name appears in 2+ controllers, read them side by side. Three copies of an 8-line method become one method on the base controller in 5 minutes — if you know they exist.

2. Same conversion logic, different code path

A web controller and a background command both had an alertToArray() method. Same 16-key array. Same computed fields. One was private, the other protected. Otherwise identical.

This one is especially sneaky because the two files serve different purposes: one handles HTTP requests, the other sends scheduled emails. No reviewer would think to compare them.

Why review misses it: Different file, different purpose, different developer, different sprint. The web controller was built first. The email command was built weeks later by copying the array format. Nobody thought to extract it because nobody knew the first one existed.

How to check:

# Find method names that appear in multiple files
grep -rn "private function\|protected function" app/ --include="*.php" \
  | grep -v vendor/ | awk -F'function ' '{print $2}' \
  | sed 's/(.*//' | sort | uniq -c | sort -rn | head -10

When conversion logic is duplicated, the fix is usually putting it on the model: Alert::toDigestArray(). Both consumers call the same method. The array shape changes in one place.

3. The model method nobody calls

Three controllers had inline sync queueing logic, 10-15 lines each creating a queue item and calling an artisan command. Meanwhile, the model already had a queueSync() static method that did the same thing. Nobody used it.

This is the pattern that bugs me most. The right approach existed. Three developers independently wrote their own version because they didn’t know the model method was there.

Why review misses it: A reviewer reads AppController::sync() and sees 12 lines of queueing logic. It works. It’s clear. There’s no obvious problem. The reviewer would need to also know about a static method on a model they aren’t looking at, and know that it does the same thing.

How to check:

# List all static methods on models
grep -rn "public static function" app/Models/ --include="*.php"

# For each one, check how many files actually call it
grep -rl "YourModel::methodName" app/ --include="*.php" | grep -v vendor/ | wc -l

# Shortcut: find ALL static calls to models across the app
grep -rn "\\b[A-Z][a-zA-Z]*::[a-z]" app/Http/Controllers/ --include="*.php" \
  | awk -F'::' '{print $1}' | awk -F/ '{print $NF}' | sort | uniq -c | sort -rn

If a model has a static method with zero or one callers, and you find similar inline logic in controllers, the logic should flow through the model method.

4. Duplicate fetch with a subtle twist

Two controllers fetched the same enrichment data: device breakdowns, search appearances, traffic metrics. Same DB query, same array shape. But one had a permission guard for shared properties, and the other didn’t. One had a different GSC API fallback scope.

The subtlety makes this pattern survive longer than pure duplicates. A reviewer who bothers to compare the two sees differences and concludes they’re intentionally different. They aren’t. The shared piece (DB query + map logic) is identical. The contextual piece (guards, fallbacks) is different. But because they’re tangled together, the whole thing looks intentional.

Why review misses it: The code isn’t identical, so diff won’t catch it. The reviewer sees minor differences and assumes they’re deliberate. The actual structure (shared core wrapped in different context) isn’t visible from either file alone.

How to check:

# Find similar DB queries across controllers
grep -rn "::where\|::query\|DB::table" app/Http/Controllers/ --include="*.php" \
  | awk -F: '{print $3}' | sort | uniq -c | sort -rn | head -10

When you find duplicated fetch logic with slight differences, extract the shared piece into a service. Let each controller keep its own guards and fallbacks. Centralize the read; distribute the context.

5. The controller that’s only fat in context

One controller was 438 lines. Three private methods (getEmerging(), getLost(), getCtrAnomalies()) were pure business logic: query ranking data, apply thresholds, compute anomalies. No HTTP concerns at all.

438 lines isn’t inherently wrong. But its peer controllers handling similar work were 120-170 lines. The relative difference signals that business logic migrated into the wrong layer.

Why review misses it: Reading a 438-line file top to bottom, it flows fine. Each method makes sense in sequence. The fat isn’t obvious until you see the controller next to its peers. “This one is 3x the size of the others doing similar work” is a relative signal. Single-file review gives you no baseline.

How to check:

# Compare controller sizes
wc -l app/Http/Controllers/*.php | sort -rn | head -15

If one controller is 2-3x the size of its peers, read the private methods. If they don’t touch $request, $response, or views, they belong in a service. Fix the primitive first, then update every consumer, not the other way around.

Why file-by-file review is blind to these

All five patterns share a trait: no single file looks wrong.

  • The duplicated method works correctly in each file
  • The fat controller reads fine top to bottom
  • The unused model method sits quietly
  • The subtle duplicate looks intentionally different

The problems only appear when you compare files: side by side, relative to peers, across code paths. File-by-file review doesn’t do that. It reads each file in isolation, asks “does this work and make sense?” and moves on.

You do need to look across files, not just inside them. grep, wc -l, and the discipline to compare rather than just read will catch most of them without any specialized tooling.

The rule

A file that passes code review can still be structurally wrong. Not because the reviewer missed something — because the problem doesn’t exist inside any single file.

Check structure, not just correctness. Compare, don’t just read.


Related: