Redundant Code Gets a Bad Rap (Part 1)

If you’ve ever done any kind of Computer Science, you’ve probably heard that redundant code is bad. Nobody ever talks about how eliminating redundancy can get you into trouble.

Let me explain what I mean with a parable. Andy is a highly-principled engineer. He knows that redundant code is bad, so when he spots these two functions in the code base…

void initPTCModuleA() {
    mHnd = generatePTCHandle();
    unlockInputChannel(mHnd);
    syncToSystemClock(mHnd);
    
    embellishWithA(mHnd);
}
void initPTCModuleB() {
    mHnd = generatePTCHandle();
    unlockInputChannel(mHnd);
    syncToSystemClock(mHnd);
    
    embellishWithB(mHnd);
}

…he immediately factors the duplicate code into a function.

void newBasicPTCHandle() {
    mHnd = generatePTCHandle();
    unlockInputChannel(mHnd);
    syncToSystemClock(mHnd);
}
void initPTCModuleA() {
    newBasicPTCHandle();
    embellishWithA(mHnd);
}
void initPTCModuleB() {
    newBasicPTCHandle();
    embellishWithB(mHnd);
}

Time goes by.

Another engineer, Bethany, discovers a rare bug and traces the bug to the common function:

void newBasicPTCHandle() {
    mHnd = generatePTCHandle();
    unlockInputChannel(h);
    syncToSystemClock(h);
}

She fixes it by changing the order of initialization calls.

void newBasicPTCHandle() {
    mHnd = generatePTCHandle();
    syncToSystemClock(h);
    unlockInputChannel(h);
}

Except I forgot to tell you something important. Bethany is on the iOS team. And initPTCModuleA() only ever gets called on Android. Bethany’s change actually introduces a bug on Android, but she doesn’t realize it because she only has iOS devices at her desk.

Calvin is on the Android team, but he hasn’t pulled in a while, because he’s working on a big UI revamp, so his next pull brings a tidal wave of new commits including the change that broke this init function. Calvin wastes hours looking for the bug, because he assumes it has to do with his UI changes. Finally, he resorts to a git bisect and finds Bethany’s commit.

Now it might seem like Bethany made this mess and she should have to clean it up, but it isn’t that simple. Maybe the real issue here is that embellishWithA() makes problematic assumptions about the code that precedes it.

Of course, that explanation isn’t going to impress Darcy the tech lead. The way she sees it, the Android build worked before, now it doesn’t, the solution was difficult to track down, and none of this would have happened if Andy had left the redundant code alone.

Sharing code between functions is great, but sharing code between projects actually requires a lot of diligence. Contracts between calling code and common code need to be very explicit, and people need to be aware that changes made to common code affect everybody. It’s virtuous to have your coworkers’ backs, but it also represents extra work. Redundant code isn’t just unambiguously bad, there are tradeoffs.