Skip to content
AtomicReps

A good name you did not choose

A lesson from The Model You Stopped Building. Play it above, or read it through below.

  • The loop kept its promise. The read-out did not.

    The common read: an object keeps things in the order I put them in, so an index built latest-first reads back latest-first

    Insertion order is not a property of the object. It is a property of the keys. The loop did exactly what its name says: it sorted by openedAt descending and inserted 7, then 1042, then 310. The object handed those three keys back as 7, 310, 1042, which is ascending numeric order and is nobody's insertion order. The oldest incident surfaced above the second-newest one, and the titles print in that same rearranged order.

    The rule is one clause of the specification, and this is the shape that hides it: own-property enumeration emits every key that is an array index first, in ascending numeric order, so a key that happens to look like an array index is placed by its numeric value at the moment it is written, not by the order it was written in. An id is a string. "7" and "1042" are strings shaped like array indices, and the object treats them as positions rather than as names.

    The triage list on the on-call dashboard has been ordered by that function since it shipped. The retro after the missed page carried an action item to add a lint rule for unsorted output, and the output was sorted. The read-out is the one step nobody wrote down.

  • The function was never wrong. The container was.

    The common read: the function is broken, so the same call on the same data returns the same wrong order whatever the ids look like

    Nothing in the function changed and the answer changed. Three characters in front of each id, the same loop, the same comparator, the same three rows, and the read-out came back INC-7, INC-1042, INC-310, which is newest first and is what the name promised on the first screen. The prefix did not repair a defect. It moved the keys out of the class the object sorts, into the class the specification enumerates in creation order.

    Read the two runs together and the fault becomes locatable. The sort ran correctly in both. The insertion ran correctly in both. The only variable was the shape of the key, which is a property of data that arrived from somewhere else, so a reader hunting a bug inside the function was searching the one region of the system where there was nothing to find, which is why more careful reading of that file does not converge on an answer.

    A fix landing in the wrong file still closes the ticket. Wrapping the read-out in a second sort, or swapping the comparator, produces a green test against a fixture whose ids were already in order, and the next id format that arrives reopens it without a line changing. The container is the repair: a Map keeps every key in creation order and has no numeric class at all.

  • The keys that jump the queue are the keys an index is made of.

    The common read: integer-like keys are an edge case, so the rule does not reach the code I review

    Name the id formats this rule catches, then read the boundary run. 9, 10 and 4294967294 came back first, in numeric order; -1, 1.5, 007 and 4294967295 came back after them. The top edge is exact and measured, not asserted: 4294967295 is one past the range and stays where it was written. A key is an array index when it is the canonical decimal spelling of an integer from zero up to 2^32 - 2, so 007 is a name and 7 is a position.

    That set is the set of database serial keys, port numbers, status codes, error codes, year buckets and anything else a service uses as an id and stores as a number. A Map has no such rule: the same loop against a Map returned 7, 1042, 310, which is the order the rows went in. Nothing in the reading of indexByLatestFirst says which container it built, because the container is one word inside the body.

    The review that approved the function ran on the function. The id format that decides the answer lives in whatever produced rows, which was not in the diff and is in no file the review opened.

  • The name was a claim about the result. The code only promised the loop.

    The common read: a name I would defend in review is a name I can read past, so an accurate name is a safe name

    Accuracy is not the axis. indexByLatestFirst is accurate about the loop it runs, says nothing about the object it returns, and a reader who trusts it is trusting a claim the function never made. Written out as a sentence and run as a check, that claim reads: the keys come back latest first. It returned false on the plain ids and true on the prefixed ones, on identical code.

    The shape to carry off this screen is a name asserting a property of the result while the code guarantees only a property of the process. indexByLatestFirst guarantees an insertion order. allShipmentsScanned, two screens from now, guarantees a quantifier. The version you can find by eye is an ordering word bound to a plain object, a latest or a sorted or a ranked sitting on something that is neither an array nor a Map, which is a promise the container was never able to keep.

    The incident doc's root-cause field reads sorting bug in indexByLatestFirst, and the sort is the one line in that file that did exactly what it says. What happened is that a claim about the result was read off a name that only ever described the loop, by a reader who has read ten thousand accurate names and holds no habit for checking one.

  • The batch was empty and the gate said yes.

    The common read: a check named allShipmentsScanned fails closed, so an empty batch cannot pass it

    Name what a check called allShipmentsScanned returns for a batch with nothing in it, then read the line under this one.

    An empty batch comes back

    The gate upstream filtered the manifest down to shipments out for delivery, and on this manifest that filter matched nothing. Batch size zero, allShipmentsScanned true, truck released. The specification says it in one line: every acts like the for-all quantifier in mathematics, and for an empty array it returns true. Vary the batch and the predicate behaves exactly as read: one unscanned row is false, one scanned row is true, a mixed batch is false. Only the empty case reads as a permission.

    This one is a transfer and it is worth naming as one, because a reader who predicted the index correctly two screens ago can still lose here: the name is shaped like a safety assertion, safety assertions are assumed to fail closed, and nothing about every fails closed. The postmortem lists the upstream filter as the root cause. The filter is correct, and it is the only component in the chain that was ever asked to be.

  • You did not read that function. You recognised it.

    The common read: I read that carefully and still missed it, so more care on the next diff is the fix

    Care was not the missing input. Reading code at speed is recognition: the name and the shape resolve to a known thing, and the body gets confirmed rather than executed in the head. Felienne Hermans describes expert programmers doing precisely that, not going line by line but saying this is the decorator pattern applied to that variable, and she is describing human beings reading code that other human beings wrote. The recognition is the skill. It is also the vector.

    Take the name away and the same function reads differently. With the identifier reduced to f there is no claim to confirm, so the reader follows the loop into the object, and the id format is the first thing anyone looks at. The measured result points the same way: a controlled study of professional developers reading production methods found that misleading names, or names that clash with their types, are worse than meaningless names like consecutive letters of the alphabet.

    None of that literature is about code you did not write. It measured people reading each other's work, and the transfer is not a finding, it is arithmetic: recognition runs on names, a name you did not choose was never checked against your model of the system, and nothing in the mechanism cares who typed it. The review comment on the diff said the names were clear, which was true, and is why the diff went through in four minutes.

  • The name passed its own review. That is what a plausible name does.

    The common read: a name that misleads is a review finding, so a careful reviewer catches it

    A misleading name does not look misleading. The controlled study that measured misleading names as worse than meaningless ones did not go looking for them: its authors recorded that they did not realise how bad the variable names were when they started, on methods drawn from production utility classes that working developers had written and shipped. Naming is subjective, and a name one developer finds meaningful is misleading to another. The author's review of their own name passes every time.

    So the finding cannot arrive through review. Name-versus-behaviour dissonance is a catalogued defect class, and the sub-class developers surveyed about it agreed on universally is exactly the one where behaviour and lexicon clash, which is also the one that keeps shipping, because the disagreement was never about whether it is bad. Reading the name does not surface it. Running the thing does.

    If your prediction on the first screen was 7, 1042, 310, the run printed 7, 310, 1042. That gap is a property of reading rather than a property of you: fluency and prediction are different operations, and reading practices the first one. The blameless timeline records the approval and the four minutes it took. It does not record that nobody in the thread could have said what the function returned.

  • The name is a claim. Two lines decide it.

    The common read: the repair is to read more carefully, so the answer is a slower review

    Slower reading buys more recognition, not more prediction. The instrument that separates the two is cheap and it is not review: write the name down as a sentence about the result, say what that sentence predicts, and run it. Both names in this lesson were written out that way in one file. Both came back broken on the first execution, on code that no reading anywhere in this lesson found suspicious.

    The check is not free and does not belong on every diff. Price it by blast radius: where an almost-right answer costs a rerun, recognition is the right instrument and reading the name is the whole review. Where a rearranged order or a vacuous true reaches something that reconciles months later, two lines are the cheapest step in the process, and the only one producing a fact rather than a feeling. The retro's action item was a naming-convention document, which every name here satisfies.

    The thing to take is not a rule about naming. It is a division: a name you can defend and a claim you have checked are different objects, and only the second costs a run. Nothing here says the names were bad. indexByLatestFirst was a good name chosen for a loop, it is still a good name, nobody who wrote or approved it did anything a reviewer flags, and the index comes back in the wrong order on every id format a service is likely to use.