DEV Community

Mahiro Hirakawa
Mahiro Hirakawa

Posted on

My duplicate-code detector flagged the check I had just written. I deleted the duplication, not the rule.

A detector in my build looks for repeated code shapes. It caught a ten-line window shared between two checks that I had written deliberately, days apart, believing them to be different.

FAIL_SCAFFOLD  dup=1  window=10 lines  faces=verdict, term
Enter fullscreen mode Exit fullscreen mode

They were not different. They were the same shape with different words in it.

Why a text diff would have missed it

The detector does not compare text. It erases string literals and normalises accessor calls to a single form before comparing:

// as written, in two files, months apart
const kind = value(row, "verdict_kind");
const unit = value(row, "verdict_unit");

const kind = value(row, "term_kind");
const unit = value(row, "term_unit");
Enter fullscreen mode Exit fullscreen mode
// what the detector compares
const _ = value(_, "");
const _ = value(_, "");
Enter fullscreen mode Exit fullscreen mode

Every identifier that made these look like two jobs is exactly the part that gets erased. What remains is the shape, and the shape was identical for ten consecutive lines.

That is the whole value of the thing. Duplication that survives a rename is invisible to grep and to review, because the reviewer reads the words.

Two ways to make a detector stop complaining

response what it changes what it costs
add an exception for these two files the rule every future duplication in them is now unreported
raise the window from 10 to 11 lines the rule every 10-line duplication anywhere is now unreported
remove the duplication the code the work of the refactor, once

The first two are one keystroke and they are how a detector dies. Not in one dramatic decision, but as a list of exceptions each of which was reasonable at the time.

The rule here is a copy ban that applies inside our own tree. It had just caught our own newest control, which is precisely the case where a rule is most tempting to bend and most worth keeping.

The fix, and the control that made it safe

The four repeated cells are now declared once and read through one map, so the per-cell lines do not exist to be duplicated:

before:  two files, four near-identical cell blocks each
after:   one declaration, one reader
         OK_SCAFFOLD faces=8/8 dup=0
         scaffold tests 67/67
Enter fullscreen mode Exit fullscreen mode

dup=0 says the duplication is gone. It does not say the behaviour survived, and a deduplication refactor is exactly the kind of change that quietly alters an output while looking tidy.

So the load-bearing check is the other one: every verdict line the build emits is byte-identical to before the refactor.

OK_ALL controls=24     all prints byte-identical to the pre-refactor run
Enter fullscreen mode Exit fullscreen mode

That pair is what makes the change safe to keep. dup=0 alone would be satisfied by a refactor that broke something; byte-identical output alone would be satisfied by doing nothing. Together they say the code changed and the behaviour did not.

What I would take from this

When a rule catches you, the first question is whether the rule is wrong, and the honest answer is usually no. I wrote the detector, I wrote the thing it caught, and my first instinct was still to look for the exception.

Normalise before comparing, if you want to find duplication that matters. The duplication people actually ship is not copy-paste; it is the same structure written twice with local names. A comparison that keeps the names cannot see it.

A dedup refactor needs a behaviour-preservation control, not a duplication count. The count tells you the smell is gone. Only identical output tells you the meaning is.

Top comments (0)