I have reviewed so much code that I usually prefer multi-line formatting by default.
Not because it looks prettier.
Because code review happens line by line. And also tools like static analysis or SQL errors are reported line by line.
PS: in most IDEs you can directly jump into the code by typing or clicking FILE:LINE e.g. in the quick-seach (typing) or in the command line (clicking)
A one-line change should ideally mean:
one thing changed
That sounds obvious, which means we apparently need examples.
The problem with compact code
Compact code often hides the actual change.
A reviewer does not review “the code” in some magical abstract universe.
A reviewer reviews a diff.
And diffs are line-based.
So when multiple independent decisions live on the same line, every change becomes harder to verify.
That is how small changes sneak through review.
Not because reviewers are lazy.
Well, sometimes because reviewers are lazy.
But often because the formatting makes the review harder than necessary.
JavaScript
Bad:
const user = users.find(user => user.id === selectedUserId && user.active && !user.deleted);
Better:
const user = users.find(
user =>
user.id === selectedUserId
&& user.active
&& !user.deleted
);
Now a reviewer can see whether someone changed:
user.id === selectedUserId
or:
user.active
or:
!user.deleted
One logical condition per line.
Primitive stuff. Somehow still rare.
SQL
Bad:
SELECT id, name, email, created_at FROM users WHERE active = 1 AND deleted_at IS NULL ORDER BY created_at DESC;
Better:
SELECT
id,
name,
email,
created_at
FROM users
WHERE active = 1
AND deleted_at IS NULL
ORDER BY created_at DESC;
Now changing:
email
to:
private_email
is one line.
Adding a filter is one line.
Removing a selected column is one line.
The diff no longer looks like someone threw SQL into a blender and called it “compact”.
Conditions
Bad:
if ($user !== null && $user->isActive() && !$user->isDeleted() && $user->hasPermission('admin')) {
// ...
}
Better:
if (
$user !== null
&& $user->isActive()
&& !$user->isDeleted()
&& $user->hasPermission('admin')
) {
// ...
}
Now each condition can be reviewed independently.
This matters when someone changes this:
&& $user->hasPermission('admin')
to this:
&& $user->hasPermission('user')
That is not a formatting change.
That is the kind of thing that becomes a production incident wearing a fake mustache.
HTML
Bad:
<input type="text" name="email" id="email" class="form-control is-invalid" value="{{ email }}" required autocomplete="email">
Better:
<input
type="text"
name="email"
id="email"
class="form-control is-invalid"
value="{{ email }}"
required
autocomplete="email"
>
Now changing required, autocomplete, class, or value is visible.
This also avoids huge horizontal lines, because apparently screens are wide but attention spans are not.
Function parameters
Bad:
$user = $userRepository->findActiveUserByEmail($email, true, false, ['admin', 'editor']);
Better:
$user = $userRepository->findActiveUserByEmail(
email: $email,
includeDisabled: true,
includeDeleted: false,
roles: [
'admin',
'editor',
],
);
This is not only easier to review.
It also prevents the classic mystery-boolean nonsense:
true, false
Lovely.
Very expressive.
Archaeologists will enjoy it.
Arrays
Bad:
$config = ['enabled' => true, 'timeout' => 30, 'retries' => 3, 'log_errors' => true];
Better:
$config = [
'enabled' => true,
'timeout' => 30,
'retries' => 3,
'log_errors' => true,
];
Now changing one value changes one line.
Adding one entry adds one line.
Removing one entry removes one line.
No fake diff noise.
No guessing.
No “wait, what changed here?”
Config blocks
Bad:
$options = ['cache' => true, 'ttl' => 3600, 'debug' => false, 'strict' => true];
Better:
$options = [
'cache' => true,
'ttl' => 3600,
'debug' => false,
'strict' => true,
];
Configuration changes are exactly where review clarity matters.
One flag can change behavior across the whole system.
So maybe, just maybe, hiding it inside a long horizontal sausage is not peak engineering.
Method chains
Bad:
$result = $queryBuilder->select('id', 'name')->from('users')->where('active = 1')->orderBy('created_at', 'DESC')->limit(50);
Better:
$result = $queryBuilder
->select('id', 'name')
->from('users')
->where('active = 1')
->orderBy('created_at', 'DESC')
->limit(50);
Now each query operation has its own line.
A reviewer can spot whether the change touched:
->where('active = 1')
or:
->limit(50)
That matters.
Especially when someone quietly turns 50 into 5000 and suddenly the database starts filing a workplace complaint.
The actual rule
Prefer multi-line formatting when a line contains multiple independent pieces of meaning.
Especially for:
js expressions
sql queries
conditions
html attributes
function parameters
arrays
config blocks
method chains
This does not mean every line must be split.
This:
$isActive = true;
does not need ceremony.
We are formatting for reviewability, not starting a whitespace religion.
Compact code is not automatically readable
Compact code can be readable when it contains one idea.
Compact code becomes a problem when it compresses multiple decisions into one line.
Reviewers need to answer simple questions fast:
- What changed?
- Is the change safe?
- Is the changed line doing one thing?
- Did the author hide behavior inside formatting noise?
Multi-line formatting helps with that:
- It reduces noise.
- It makes intent visible.
- It makes review sharper.
Final thought
Readable diffs beat clever formatting.
Every time.
Top comments (0)