Today I intentionally copied a bug

September 21, 2022
Guessing what code is meant to do is hard work.

Today I was rewriting a function. The function makes an HTTP request with a form-encoded request body. But after preparing the body of the reqeust, it runs it through a regular expression to replace /%5B\d+%5D=/ with []=.

There’s no comment explaining why this is done, so I have to guess.

I have a couple guesses.

  1. The API we’re calling doesn’t like the array indexes (the \d+ part in the regular expression).
  2. The API we’re calling doesn’t properly unescape the %5B and %5D sequences.
  3. Maybe both?

I really suspect it’s #1. If the case were #2, surely we’d just replace all occurrences of %5B with [ and %5D with ], not only the occurrences with digits between.

But that leaves the question: Why don’t we replace matches with %5B%5D=? Why do we un-escape these characters, too?

So what did I do?

I copied this dubious behavior into my new function. Even though it’s sloppy and confusing, it’s probably harmless. And removing it is sure to cause problems, since I don’t know exactly what behavior it’s trying to correct for.

What’s the lesson? Two, I think:

  1. Make your code obvious. If you can’t explain why the code exists with the function name, use comments.
  2. Rewrites are dangerous. Code, no matter how poorly organized and ugly, contains months or years of accumulated bug fixes and wisdom. As tempting as it is to rewrite, do it carefully, lest you re-introduce bugs you don’t understand!
Share this

Related Content

Hair cuts and peekaboo

How often do you come across code that look completely superfluous? Like a towel to a toddler during a haircut.

Regular Expressions Are the Best! s/Best/Worst/

Regular Expressions. Ya love 'em or ya hate 'em. But it shouldn't be so black-or-white. Here's when they do, and don't make sense.

“Greenfield” doesn't exist in agile projects

I've worked on a number of greenfield projects, but there's a problem: A greenfield project is only greenfield for about a week.