Skip to main content
ArticlesProjects

Every Feature Touches Ten Files

Ten files open for a one line change is either layering working correctly or one idea smeared across a codebase. The count does not tell you which, and git history does.

Somebody asks for a small change. Orders need to record which currency they were paid in. It is one column and one field, and by the time it works you have edited a migration, the model, a factory, two form requests, an API resource, an export, a search index, an admin view, and four tests.

The pull request is eighty lines across thirteen files and none of it was hard. It was just long, and slightly tedious, and by file nine you stopped reading carefully.

The conclusion everybody reaches in the retro is that there is too much boilerplate, and the fix is to have fewer layers. That conclusion is sometimes right. The problem is that the same file count comes out of two completely different situations, one of which is your architecture working exactly as intended, and you cannot tell them apart by counting.

Two shapes, one number

Here is the other thirteen file change. A new feature: customers can request a refund.

app/Http/Controllers/Api/RefundController.php new
app/Http/Requests/StoreRefundRequest.php new
app/Actions/Orders/RefundOrder.php new
app/Http/Resources/RefundResource.php new
app/Policies/RefundPolicy.php new
database/migrations/..._create_refunds_table.php new
database/factories/RefundFactory.php new
tests/Feature/Api/RefundTest.php new
tests/Unit/RefundWindowTest.php new
routes/api.php edited

Ten files, nine of them new, one edited. Every file exists because the feature needed a different kind of thing: a way in, a validation of the way in, a decision, a way out, a permission, a place to keep it. Nothing that already existed had to change its mind about anything.

Now the currency change again, written out the same way.

database/migrations/..._add_currency_to_orders.php new
app/Models/Order.php edited
database/factories/OrderFactory.php edited
app/Http/Requests/StoreOrderRequest.php edited
app/Http/Requests/UpdateOrderRequest.php edited
app/Http/Resources/OrderResource.php edited
app/Exports/OrdersExport.php edited
app/Search/OrderIndexer.php edited
resources/views/admin/orders/show.blade.php edited
tests/* edited

One new file. Nine edits. And the edit is the same edit nine times: the word currency, added to a list of fields.

That is the difference, and it has nothing to do with the number ten. In the first change you added files. In the second you edited files, and what you edited in each of them was a restatement of the same knowledge.

Martin Fowler named this one shotgun surgery in Refactoring, and the name is good because it describes the feeling accurately. You are not doing one difficult thing, you are doing one easy thing in nine places and hoping you did not miss the tenth.

The knowledge is written nine times

Look at what those nine files actually contain.

app/Models/Order.php
protected $fillable = ['reference', 'total', 'currency', 'purchased_at'];
// app/Http/Requests/StoreOrderRequest.php
'currency' => ['required', 'string', 'size:3'],
// app/Http/Resources/OrderResource.php
'currency' => $this->currency,
// database/factories/OrderFactory.php
'currency' => 'gbp',

None of those lines is wrong. Every one is idiomatic Laravel, and if you deleted the class that held it you would break something real. But between them they are answering the same question four times: what is an order made of?

That question has one answer, and it is currently stored in four places that do not know about each other. So the failure mode is not that the change is long. It is that the change can be done incompletely and still pass CI, because nothing anywhere asserts that these four lists agree.

Here is the tell, and you can go and look for it in your own codebase in about a minute. Find a field that is in $fillable and missing from the resource. Or in the request rules and missing from $casts. In any Laravel application over about two years old there is usually at least one, and it is almost never deliberate. It is a field somebody added to seven places when there were eight.

Ask git, because git already knows

The nice thing about this symptom is that it leaves a record. If two files keep changing in the same commit, that is not a hunch, it is history.

Terminal window
# how often each file changes
git log --name-only --format='' -- app/ | grep . | sort | uniq -c | sort -rn | head
# which files keep changing together
git log --name-only --format='%n' -- app/ \
| awk 'NF==0{if(n>1&&n<12)for(i=1;i<n;i++)for(j=i+1;j<=n;j++)print f[i]" + "f[j];n=0;next}{f[++n]=$0}' \
| sort | uniq -c | sort -rn | head

The second one pairs up every file in a commit with every other file, then counts the pairs. The n<12 is there to throw away commits that touched half the repository, because a formatting sweep or a big rename will otherwise couple everything to everything and drown the signal.

Two things matter in that output, and the second is the one people miss.

A high pair count between files in the same feature is expected. A controller and its test change together, and that is not a finding. What you are looking for is a high pair count between files in different features, or between files in different layers that should have been able to move independently.

And you have to read the count against how often each file changes on its own. Two files that changed together nine times out of nine are welded. Two files that changed together nine times out of sixty are just both busy. The first git log gives you the denominator.

I ran this against a repository of my own while writing this and the top pair was two paginated archive routes that had changed together four times out of nine. They are not coupled by accident. They are two copies of the same idea, and every time the idea changed, both copies needed the same edit. That is the same defect as the currency field, in a different costume.

When ten files is correct

Now the part where I argue against the obvious fix, because the obvious fix here is worse than the problem.

The reflex after a change like this is to collapse layers. Delete the resource and return the model. Delete the form request and validate in the controller. Delete the action and put it back in the controller too. That does genuinely reduce the file count, and for a while it feels like progress.

It does not fix anything, because the file count was never the problem. Those layers were not duplicating knowledge, they were separating concerns that change for different reasons. The serialisation of an order changes when a client needs a different shape. The validation changes when the business rules change. The persistence changes when the schema changes. Merging them means all three now change together, which is the actual defect you started with, applied to more of the codebase.

The refund feature above touched ten files and none of them were a problem. If your answer to “how many files did that take” is a number you find upsetting, get a different question.

The question that works: did I write the same thing more than once?

What it costs to fix

The fix for a smeared field list is one place that knows the shape.

final readonly class OrderData
{
public function __construct(
public string $reference,
public int $total,
public string $currency,
public DateTimeImmutable $purchasedAt,
) {}
}

Now the request builds one, the action takes one, the resource reads one, and adding a field is a change to a constructor that will not compile in the places that need updating. PHPStan on a decent level will tell you before CI does.

This costs a class per shape, and it is not free. On a model with three fields and one consumer it is ceremony, and I would not add it. It starts paying when the same field list is written in four places, which is exactly the moment it stops being obvious that they have drifted.

There is also a real cost people do not mention: a data object gives you a second place where the shape lives, right up until you delete the duplication from the other places. Half-finished, this makes things worse rather than better. If you are going to do it, do it for one model completely rather than for six models partially.

What to do on Monday

Run the two git commands against app/. Take the highest pair where the two files sit in different features and ask what knowledge they share. It is usually a list of fields, a status enum written as strings in several places, or a rule about who is allowed to do something.

Then take the model you are most tired of editing and diff its $fillable against its resource. If they disagree, you have found the cost of the smear in its most concrete form: a field the API does not return, that nobody noticed, because eight places out of nine was enough to make the tests pass.

Next in this series: the model nobody wants to touch, and why counting its lines is the least useful thing you can do to it.

Part of a Series

Why Is This Hard To Change?

You are reading Part 2 of 7 in this learning series.

View Full Series

Share

XLinkedIn

Related

Keep Reading

All posts →