I'm sure as hell this antipattern has a name

wft

Someone at my company wrote a piece of code that solved a problem. This piece of code was a clever one, and the solution for the problem was elegant and performant, except for one thing: it didn't work well on production. Apparently, replication in MySQL didn't play well with creating and dropping tables all the time.

So he rewrote the code to go a more robust, if boring, route, but at the end of iteration he left the old code in place, while writing new code alongside and employing a bunch of if's to switch the logic. Like,

sub dofoobar_oldmethod {
    if (!$globalconfig{useoldmethod}) {
        return dofoobar_newmethod(@_);
    }
    ....
}

all over the place, the rationale being, "we might want to use this method elsewhere," or, "this will be cleaned up after the new way is tested enough."

Sure enough, the old way falls into disuse as the new one is proved. The "switch" is hardcoded, not documented, and happily forgotten about. It's just that other programmers, when they find something they need to fix in this module, must fix it in both places, because they can't tell right away which part of it is used and which is not.

In another place, I even found three variations of doing the same thing, of which maybe one worked (switchable of course, with multitudes of copious ifs sprinkled all around, but ultimately hardcoded).

What I found disturbing is that this was the way this colleague reasoned all the time. The whole concept of polymorphism where you use derived objects for the same damn thing, somehow failed to meet him on the way of his over-a-decade-long career. Another thing was the extreme reluctance to remove the dead code. What the heck, it's all there in the git repository. And it's not like he's averse to version control, I think it's just some kind of emotional attachment to some clever, if imperfect, solutions, which must be hoarded and supported even when totally unused.

Mikael_Svahnberg

I'd call it #ifdef hell, if you need a pattern name for it.

At one company I worked with where they used (in some sense legitimately) ifdefs for their different configuration flags, someone had estimated that there were roughly 64000 unique combinations of flags -- making debugging a nightmare. They even had a tool that essentially gave them back the code as it would look like after the preprocessor had run (sans all the #includes), so that they could do meaningful debugging on that particular flag configuration.

Suggestion: Create a module "LostPuppies" where he can bury his darlings, and keep them out of the rest of the codebase. As you say, git can remember them for him, should he need them again.

aliceif

Why not just

someType doStuff(parms ...){
    return useOldMethod ? doStuff_old(parms) : doStuff_new(parms);
}

?

That would make the ifs only appear in one place

[Better than what you have, still not good, though](#tag), [Sorry, I don't speak Perl, so have some pseudo-C](#tag)

wft

Still has this weird, hard-to-track logic buried in.

Instead of having two classes, with doStuff() in each one.

blakeyrat

@aliceif said:

That would make the ifs only appear in one place

Because that doesn't solve the problem of having tons of useless dead code littering the codebase.

You're focusing on a leaf. There's a tree. There's a whole forest. Ignore the leaf.

Jarry

@blakeyrat said:

Ignore the leaf.

burn the forest?

blakeyrat

Forest: Gobs of dead code everywhere
Trees: Using if() to mark sections of dead code
Leaves: You can use if() a slightly better way

dstopia

Sounds similar to a problem I'm facing right now with someone who created a bunch of code to implement a Mediator pattern and only today I dug into it to realize it is doing literally nothing and never gets called. Like I could proceed to delete the whole thing and the application would keep on running no problem. There's about 8 or 9 classes involved in this piece of shit, and when I asked around I got blank stares.

Cargo cult programming is the worst.

antiquarian

@Jarry said:

burn the forest?

That's what I would do. I'm pretty sure that's not the only anitpattern present in the code.

cartman82

If you need a bunch of if-s all over the place, sounds like this functionality isn't properly encapsulated. Ideally, you'd have a common interface IMethodToMessWithData and two interchangeable implementations.

Of course, in reality this is often just a pipe dream, and you can't neatly wrap everything up into its own pretty little package. In that case, dump the old stuff. Let vcs remember the beauty that never was.

Arantor

What's the name for the antipattern of 'we don't have VCS but we'll implement it by keeping old copies of functions in our files just renamed to _old or maybe we'll just comment it out?' I'd love for VCS to remember that beauty that never was.

powerlord

I'm being lazy and looking on Wikipedia for anti-pattern names.

Apparently it's referred to as Boat anchor:

http://en.wikipedia.org/wiki/Boat_anchor_(metaphor)#Software

Jaloopa

In a shop that does have VCS:

#region Commented Out code for using multiple network cards