Nobody Wants To Touch That Model

There is one in every codebase. Usually User, sometimes Order, occasionally something with a name that made sense in 2019. It is the file that comes up in code review as “we should really do something about that”, and the something never gets specified.

People have started routing around it. A new feature that logically belongs on it gets built next to it instead, because touching it means running the full suite and reading four hundred lines to find out whether the thing you added collides with something. That avoidance is the actual cost, and it is much larger than the file.

So the first question is what to measure, because the number everybody reaches for is the wrong one.

Line count measures the wrong thing

The reflex is to open the file, see nine hundred lines, and conclude that nine hundred is the problem. It usually is not.

A nine hundred line model can be completely harmless. Forty relationship methods, thirty casts, and a long $fillable, on a table that has a lot of columns. Boring to scroll past, and it changes for exactly one reason: the schema changed. Nobody is afraid of that file.

A hundred and twenty line model can be the one half the team will not go near. Five public methods, referenced from forty files across nine different areas of the application, so any change to it has a blast radius nobody can hold in their head.

Those two files are nowhere near each other on a line count, and they are the wrong way round.

What you actually want is two numbers, and neither is length.

Reasons to change. How many separate concerns would send someone to edit this file? Schema, billing, notification preferences, permissions, search indexing, that one report. If the answer is one, the size does not matter. If the answer is six, the size does not matter either, and it is a much worse file.

Fan-in, by area. Not how many files reference it, but how many different parts of the application do. A model referenced two hundred times from one module is a module’s model. A model referenced forty times from nine modules is a junction, and every change to it is a change to nine things.

Getting the numbers

Both of these come out of grep, which is the right level of tooling for a question you are asking to decide whether to bother.

# which areas of the app reach for this model
grep -rl "\bWorkspace\b" app/ --include=*.php \
  | grep -v "app/Models/Workspace.php" \
  | sed 's|app/||;s|/.*||' \
  | sort | uniq -c | sort -rn

That gives you something like 13 Http, 9 Domain, 2 Actions, 1 Jobs, 1 Console, and the shape of that list tells you more than the total. One tall bar is a model that belongs to a feature. A flat spread across everything is a junction.

For reasons to change, there is no command. You read the public methods and put a feature name next to each one, and the number of distinct names is your answer. It takes about five minutes and it is the most useful five minutes in this article.

grep -n "public function\|protected function scope\|public static function" app/Models/Order.php

If two people do this independently and disagree about which feature a method belongs to, that method is worth talking about, because it means the method has more than one audience.

Extracting traits does not fix this

Here is the move that everybody makes, and it does not work.

final class User extends Authenticatable
{
    use HasApiTokens;
    use HasBilling;
    use HasNotificationPreferences;
    use HasTeamMemberships;
    use HasSearchIndexing;
}

The file is now forty lines. The pull request that did it looks enormous and productive. And absolutely nothing has changed.

A trait is compiled into the class at definition time. Every method that was on User is still on User. Every consumer still receives all of it, the fan-in is identical, the reasons to change are identical, and the blast radius of editing HasBilling is exactly the blast radius of editing those methods when they lived in the model.

What has changed is that the methods are now somewhere else, so reading the class no longer tells you what it does. You have taken a file that was honest about being large and made it dishonest about it. Traits also share a namespace for properties, so two traits can quietly collide on $appends or a boot method, which is a class of bug that did not exist before the refactor.

I am not saying traits are bad. HasFactory and SoftDeletes are framework behaviour being mixed in, which is what the mechanism is for. What does not work is using them to make a coupling number smaller by moving text between files, and that is nearly always what an App\Models\Concerns directory turns out to be.

The test is simple: after the refactor, can anything depend on less than it did before? If the answer is no, you moved text.

The invisible version

The worst of this does not appear in the model at all.

final class OrderObserver
{
    public function saved(Order $order): void
    {
        if ($order->wasChanged('status')) {
            SendStatusWebhook::dispatch($order);
            $order->customer->notify(new OrderUpdated($order));
        }
    }
}

Now $order->save() sends a webhook and an email. Nothing at the call site says so. A test that creates an order to check a permission has to know to fake two things it never mentions, and when someone adds a third side effect to the observer, every test that saves an order acquires a new dependency without a single test file being edited.

This is where the numbers from part one come from. If your suite is full of Event::fake() and Bus::fake() in tests that are not about events or buses, that is not test hygiene. It is the observer, being paid for one test at a time.

Model events are genuinely useful for things that are about persistence: touching a timestamp, keeping a slug in sync, cascading a soft delete. They are a bad place for anything a reader would want to know happened, because the whole mechanism is designed so the reader does not have to know.

What the fix actually is

The fix is not to make the file smaller. It is to reduce the number of reasons it changes, and there are only really two moves.

Take the decisions off it. A method that decides something belongs where decisions live, and the model becomes the thing that knows how to be stored. That is the whole argument of part one in a different place: $order->canBeRefunded() reaching for now() and the relationship graph is a rule wearing a model’s clothes.

Make the side effects explicit. Move the observer’s work to the place that caused it. If refunding an order sends a webhook, the refund action sends the webhook. It is one more line at the call site and it removes an entire category of surprise.

What I would not reach for first is a repository interface with one implementation in front of Eloquent. It shows up in every discussion like this one, it adds a layer and a name and a binding, and in return you get the ability to swap out a persistence layer you are not going to swap out. If you have a second implementation, or a genuine need to test without the database that the two moves above did not solve, fine. Otherwise it is indirection you pay for every time you read the code and never collect on.

When to leave it alone

Fan-in is not always reducible, and it is important not to treat it as a defect on its own.

User is referenced everywhere in most applications because a user is genuinely involved in everything. You are not going to fix that, and trying to will produce something worse than the problem. The same is true of a tenant model in a multi-tenant application: it is a junction because the domain has a junction in it.

The question is never “how many things reference this”. It is “how many reasons does this have to change”. A model that fifty files depend on and that only changes when the schema changes is a stable dependency, which is the good kind. Depend on it freely.

The one to worry about is the model that many things depend on and that six different features have a reason to edit. High fan-in and high reasons-to-change together is the combination that makes people afraid of a file, because every edit is a change to a thing that everything is standing on.

What to do on Monday

Run the area breakdown against the model you like least. Then spend five minutes writing a feature name next to each of its public methods.

If you end up with one feature name, you are done, and the file is fine no matter how long it is. Go and be annoyed about something else.

If you end up with five, pick the one that is furthest from persistence, which is usually a method with a verb in its name and an if in its body, and move that one decision out. Not all five. One. Then look at whether anything can now depend on less than it could before, and if the honest answer is no, put it back.

Next in this series: the queued job and the controller that were supposed to do the same thing, and have quietly disagreed since March.