Originally published on decentphp.dev — part of the Writing decent PHP course. View this lesson on the original site.

Names are cheapest to fix during review

Once a name lands on main, it starts propagating. It appears in new callers, new tests, new documentation, and new conversations. Renaming later is always possible (lesson 2.12) but it is never as cheap as catching the problem in a pull request before merge.

Code review is where naming gets its last easy chance. This lesson is the reviewer's playbook: what to look for, what to ignore, what to block on, and how to give feedback that actually helps.

The goal is not to "win" a naming debate. The goal is to make the code readable for the person who opens this file in six months — which might be the reviewer, the author, or a new joiner none of you have met yet.

Before

A PR review comment stream that is slow, unhelpful, and personal.

Reviewer: "I don't love this name."
Author:   "What would you call it?"
Reviewer: "Hmm. `doThing` maybe?"
Author:   "That's worse."
Reviewer: "idk it just feels off"
...(24 hours pass)...
Author:   "Can we merge? Blocker for the sprint."
Reviewer: "Fine, approve."

Nothing useful happened. The name stays. The reviewer feels ignored. The author feels nagged. And the name is now on main, where changing it is a whole separate PR.

After

The same feedback, written so the author can act on it in under a minute.

Reviewer (comment on line 42):

  `doThing()` doesn't tell me what it does — a reader has to open the
  body to find out it sends the welcome email and queues the audit log.

  Would `sendWelcomeAndAudit()` work? Or split into two calls:
  `sendWelcomeEmail()` and `recordSignup()`?

  Happy with either, blocking on the name because this is a public
  method on UserService and we'll read it a lot.

Three things changed:

  1. the reviewer said which property the name fails — a reader has to open the body
  2. the reviewer offered concrete alternatives — the author isn't guessing what would satisfy them
  3. the reviewer stated whether they're blocking and why — "public method, read a lot" is a real reason

The author now has a one-minute fix and a clear path to merge.

Why this version is better

  • the feedback is specific, not a feeling
  • the author has at least one viable rename in hand
  • the reviewer's position (block vs suggest) is explicit
  • the conversation ends on this comment instead of spiralling for 24 hours

The reviewer's first-read test

Before writing any comment, do the read the author didn't get to do: open the diff cold and read it once, top to bottom, as if you landed on it from git blame six months from now.

Ask three questions as you read:

  1. Can I tell what this code does from the names alone? If every name is doing its job you shouldn't need to read the body to follow the flow.
  2. Do the names lie? Any validate* that mutates, any get* that writes, any is* that has side effects, any process* or handle* that is actually doing something specific.
  3. Is every name at the right level? Variable names that describe intent (lesson 2.1), function names that describe behaviour (lesson 2.4), class names that describe a single concept (lesson 2.6).

If the first read feels smooth, you can skip most naming comments and focus on the actual changes. If the first read made you stop and re-read, that's the signal — there is a naming problem worth raising.

What to block on vs what to suggest

Not every naming opinion is worth holding up a PR. Use roughly this split:

Block on names that will cause real harm:

  • names that lie about behaviour (lesson 2.11's territory)
  • placeholder names (doThing, handleStuff, process) on public or widely-called code
  • names that contradict the domain language (lesson 2.8)
  • booleans that read as actions (updateUser returning a bool), or actions that read as booleans (isSaved that saves)
  • type-shape names ($userArray, $orderList) on parameters or return types in a new public API

Suggest and accept whatever the author decides:

  • subjective preferences between two reasonable names (fetchInvoice vs loadInvoice when both are honest)
  • naming style inside a private method or local variable that isn't public API
  • alphabetisation, ordering, or length preferences
  • naming that matches an existing (imperfect) pattern in the surrounding file — worth a separate rename PR, not this one

Ignore completely:

  • names you personally wouldn't have chosen but that are clear, honest, and consistent
  • names on temporary or scratch code that will be deleted before merge

This split keeps reviews fast and focused on real readability risks instead of preference wars.

Context changes what "good" looks like

The same name can be fine in one place and wrong in another. A reviewer has to weigh the location.

Name Local helper in a 30-line private method Public method on a core domain class
process($x) fine if the surrounding 10 lines make it obvious block — every caller will have to open the body
$data fine for a one-line transform block on a constructor parameter
tmp fine as a two-line swap variable block on anything that lives past the next function
$u fine in array_filter($users, fn ($u) => $u['active']) block on a long function parameter

Reviewers who apply rules uniformly across the whole codebase end up blocking on things that don't matter. Reviewers who weight context correctly catch the problems that do.

Giving naming feedback that lands

A good naming comment has three parts:

  1. Why the current name fails. Be specific: "doesn't say what unit", "sounds like a query but writes", "same word as Customer used somewhere else to mean something different".
  2. At least one concrete alternative. Even if the author rejects it, a concrete option moves the conversation forward faster than "pick a better name".
  3. Your stance: block or suggest. "Blocking because this is public API" or "non-blocking, up to you" saves a round trip.

If you can't produce all three parts, the comment is probably not ready to post. Either dig until you can, or drop it and focus on a different issue.

Self-review before opening the PR

The cheapest naming feedback is the feedback you give yourself before the PR goes up. Before you click create pull request, read your own diff cold and ask:

  1. does every new name pass the "read-aloud" test from lesson 2.10?
  2. does every function name still describe everything the function does?
  3. would a new joiner with no context understand what this code does?

Catching a name on your own diff costs 30 seconds. Catching it after a reviewer pushes back costs a round trip. Catching it after merge costs a separate rename PR and a legacy slug map.

Self-review doesn't replace peer review — it just means the peer reviewer's time is spent on the names you genuinely couldn't see yourself, not on the obvious ones you would have fixed if you'd read the diff once.

From Writing decent PHP — Naming variables, functions, classes, and files well