The cost of not repeating yourself

The most reflexive comment in code review is some version of "can we DRY this up?" It's almost always well intentioned, it's usually approved without much thought, and every few years it quietly ruins a codebase.

I want to tell you about one function. It's fictional, but you have one like it. Everyone does.

One function to rule two documents

An online shop needs to generate two PDFs: an invoice and a receipt. Someone writes them as two separate functions. They look nearly identical. Same header, same table of line items, same totals block at the bottom. A reviewer points this out, and the author agrees, because two hundred duplicated lines feel embarrassing. So the two functions become one:

generateDocument(order, { type: "invoice" | "receipt" });

This is a good day. The diff is negative, the tests pass, everyone moves on.

A month later, the warehouse team needs packing slips. A packing slip is basically the same document without prices. It would be silly to write it from scratch when generateDocument already exists, so it grows an option:

generateDocument(order, { type, showPrices: false });

Then finance needs credit notes, which are like invoices except the totals are negative and printed in red. Another option. Then a big customer negotiates their own header layout into the contract. Another option. Then someone notices receipts in Germany need a different tax breakdown, and rather than an option, this one becomes an if statement buried in the totals logic, because the person fixing it had twenty minutes before standup.

Eighteen months in, the signature looks like this:

generateDocument(order, {
  type,
  showPrices,
  isCreditNote,
  headerVariant,
  taxMode,
  footerText,
  legacySpacing, // do not remove, ask Kelly
});

Nobody knows what legacySpacing does. Kelly left in March.

The bug that makes it real

Here's where it stops being an aesthetic complaint. A bug report comes in: credit notes are showing the tax line twice. The fix is obvious, three lines in the totals section. But those three lines run for all six document types, and only one of them is broken. So now the person fixing a credit note bug is responsible for the rendering of invoices, receipts, packing slips, and two customer-specific variants they've never seen, some of which have no tests, one of which belongs to a team that got reorganized away.

They make the fix anyway. Two weeks later, German receipts are wrong.

Notice what the abstraction actually did here. It was supposed to mean "fix a bug once, and it's fixed everywhere." In practice it means "touch anything, and you might break everything." Those are the same property. Whether it helps or hurts depends entirely on whether the call sites genuinely need identical behavior, and these never did. An invoice and a receipt aren't the same document. They just looked the same on the day someone compared them.

That's the trap. DRY, as originally stated, is about not duplicating knowledge: a tax rate, a business rule, the definition of what makes an order shippable. Somewhere along the way it got flattened into "no two blocks of code should look alike." But code that looks alike today and code that must change together forever are very different things, and only the second one deserves a shared home.

The fix nobody wants to suggest

The way out of this mess is to inline the function. Copy the body into each document type, delete the options one call site at a time, and end up with six boring functions that repeat themselves.

Almost nobody proposes this, for reasons that have nothing to do with engineering. Suggesting copy and paste in a code review feels like admitting you never read the books. The line count goes up, which looks bad in the diff. And if the unit tests were written against generateDocument itself, the refactor turns the whole suite red, so the person doing it also gets to explain why coverage dropped. It's easier to add option number nine.

But run the actual comparison. The cost of duplication is that when you fix a bug in one copy, you have to remember the others exist. That's annoying, and grep mostly solves it. The cost of the wrong abstraction is that every change to any document type requires understanding all of them, forever, and the understanding required grows with every option added. One of these costs is flat. The other one compounds.

What I'd actually do differently

A few habits fall out of this, none of them clever.

Wait longer before merging similar code. Two occurrences is a coincidence. The old rule of thumb says extract on the third, and even then, only if the copies have been changing for the same reasons. Similar shape is not evidence of a shared concept. It's often evidence that you don't understand the problem yet, and the duplication is where that understanding will eventually show up.

Test the outputs people care about, not the helper. If your tests say "the invoice renders correctly" and "the packing slip has no prices," they'll survive any refactor, including inlining the shared code. If your tests say "generateDocument handles taxMode B," they've welded the abstraction in place, because deleting it now means deleting the tests too.

Make deletion a normal move. Teams celebrate extracting an abstraction and treat unwinding one as failure cleanup. It should carry the same status. A comment saying "this helper has too many flags, I'm splitting it back apart" is a healthy thing to find in a pull request, and the reviewer who approves it is doing more for the codebase than the one who first asked for the DRY-up.

There's a broader point hiding under the shipping-label example. Best practices are compressed experience: someone got burned, drew a conclusion, and passed the conclusion along without the burn. The next generation receives "don't repeat yourself" with no memory of which repetitions actually hurt, applies it to everything, gets burned in the opposite direction, and compresses that into its own slogan. The only way I know to step out of the cycle is to keep asking what a given practice trades away. Every principle is a trade. If you can't name what you're giving up, you're not following the principle. You're just repeating it.