There is an infinity of stuff I don't want my program to do. How do you decide what to put in the else branch?
–
Paul ButcherMay 17 '11 at 14:48

4

I have used this before to test portions of code that would be difficult to engineer a route to in normal operation (then changing the condition back for release) but I can't see any benefit from intentionally writing this code in from the start:- it confuses the reader, it bloats the code and it definitely won't speed it up!
–
Matt WilkoMay 17 '11 at 16:12

1

Just a comment that code is not the place to store past code changes. That's what source control is for.
–
funkymushroomMay 17 '11 at 16:20

If you use any sort of SCM dead code is rarely useful. You should delete it instead and if you ever need to see the code for logic 2 you retrieve it from the SCM repository.

Edit:

If you have dead code and are maintining other parts of the code, you might try and get usages of some code which might turn up the dead code. If someone else is doing the maintainance they might not know its actually dead code and even if they do, they most certainly won't know why it is still there.

If you then figure a method could be deleted since it is not used by "live" code but only by dead code, you'd have to either change (and most likely break) your dead code or make the other method dead code itself.

In the end you'd most certainly end up in some form of maintainance hell.

Thus, the best code is deleted code, since it can't produce wrong results or distract maintainers. :)

No, that is bad, because it clutters your code and reduces maintainability.

Usually, there is a clear reason to prefer one of the alternative "logics". This reason (and the existence of a rejected alternative) should be documented in a code comment. In the relatively unlikely event that the reason becomes invalid, the alternative code can be retrieved from repository history (if it was the previously preferred, implemented solution), or fleshed out and implemented using the full current knowledge and requirements (if it's just a vague idea).

You should keep a record of code changes somewhere so if the new code doesn't work, you can compare them. This should always be in source control. I assume you already do this. If not, do everything you can to get SOME form of source control. If you absolutely have to do without (and I have never in my life heard of an occasion when this is a good idea), at least make regular back-ups of the state of the source readily accessible.

Assuming you're doing #1, so you can recover dead code if you need to, DON'T keep it in the live code long term. It will just be confusing, add extra complexity for no value, require maintenance, get out of sync with the live code and mislead people in future, etc, etc.

That said, there are specific situations where a compile-time switch between two code paths is reasonable. While you're developing the new code, and immediately afterwards, it may be conveninet to have both, so you can easily switch between. If you're likely to have to switch back, or to add an external configuration option, an if based on a constant gives those a reasonable upgrade path. So, like many things -- if it's solving a particular problem, do it. If not, avoid it.

The reason I assume poeple do it too much is: from having doubts (often correctly) that people will actually read the source control history if they have a problem; from being scared the new code won't work and wanting an easy reversion option. A compromise to try to fulfil both of you may be to put in a comment "Changed to calculate ... on (Date). If any problems, see old version in source control" or similar.

No, it's not useful. That else block does not fulfil any purpose. If you are unsure which implementation to use, comment it out, make seperate classes or save it elsewhere. Also, mostly you have - or at least should have - a local or remote history of your source files.

The dead code should get removed by the compiler if the condition depends on a compile time constant, so technically it wouldn't hurt to keep it in. However I prefer to rather comment it since this improves the readability of the code.

If you want to be able to quickly switch between two code alternatives you can use the following convenient comment construct:

//*
alternative 1 is active
/*/
alternative 2 is commented out
//*/

if you remove only the first / in the first comment line it becomes:

/*
alternative 1 is commented out
/*/
alternative 2 is active
//*/

With this you can switch between the alternatives by just adding or removing a single / in the code.

This may look a bit strange at first but once you got accustomed to it you'll easily recognize it as some kind of pattern.

You can even chain this and thus switch multiple blocks at once with a single char:

//*
first block of code for alternative 1
/*/
first block of code for alternative 2
/*/
second block of code for alternative 1
/*/
second block of code for alternative 2
//*/

There are some very rare occasions when the old code, and the fact that it's been replaced, really should stay in the source code so that future programmers are warned about something that's counter-intuitive. In which case, there also needs to some kind of commentary explaining why it's still there and what happened.

As always, writing programs that are easy to maintain is about making things clearer, not just following hard and fast rules. If leaving the dead code in makes it easier to understand what's going on, then leave it in. If not, then take it out.

It'll be ignored by the compiler. However I agree with you and would just remove the else statement. Assuming you're using version control, you'll still be able to see the old code by viewing the history for the file.

This is probably a personal opinion, but I find dead code distracting when maintaining a piece of code. For any given task, you always have more than one way to accomplish it, so you have to choose one. Do your research, evaluate the algorithms, and choose one. After this, the code doesn't need to contain any alternative methods inside it.

If there are two very strong contenders, write up a small note about the alternative, and stick it in a project wiki, or somewhere else that contains project documentation. If you want, you can put a one line comment that refers to this document, and outlines why you may want to go read it.

The simple answer is a simple no. Dead code attracts confusion, and opportunities for bugs to occur. As soon as you type in a character into the code, there is a risk. So don't add any more than you have to.

The one time I had to write something similar was as a work around for a compiler bug, it was even recommended by the compiler vendor to use this work around.

Do you test dead code you write? Do anything to confirm that it's probably good? If not, get rid of it.

For future code changes, are you going to verify that the dead code still works? If not, get rid of it.

You don't want bad code in your programs, even if it isn't used. Having it there suggests that it can be used. You usually don't want to maintain two versions of code if you don't use both, because it's extra work, and since it feels pointless most people won't do a good job on the dead code.

There may be reasons to keep commented-out code (or, in C or C++, #ifdefed out code), but this should be accompanied with comments as to why it's still there and what purpose it serves.

Note that in Java, the following won't even compile, due to unreachable code:

int someFunc() {
return 10;
int x = 12;
return x;
}

Ideally, if there is something wrong with your code, you want to find it as early as possible, rather than letting it go to production and be found by your users.

If a class of error can be found by your compiler, then let the compiler find it. IMHO, what someone is doing by writing dead code in the manner you describe, is attempting to circumvent that compiler error, allowing any problems that it may cause to be surfaced at runtime.

You may argue that dead code can't be reached, so can't cause problems, but something may go awry in commenting, bracing and indentation that might make it happen.

The reason why people comment out some old logic is because they are afraid to make big changes to the code. And indeed, realizing a week later that the old code was actually correct and now you have to write it from scratch is a bitch. But that's what source control is for. If you decide to change some logic, don't afraid to lose the somewhat working present code. Remove it and let your SVN/Git/TFS take care of versioning for you.

If it's not the case and you just produced two different pieces of logic to do something because you don't care about YAGNI or DRY, then just make sure you put it some place where people can use it. Maybe throw a strategy pattern there if you feel like it. Just don't do these "if .. else" things, it's bad design and a pain to maintain. If you really think that some code has right to exist, make sure you make it possible to use it.

Another look at it is that basically most professional programmers agree that the size of the code is the enemy. Take a look at these blog posts: The best code is no code at all by Jeff Atwood, Code's worst enemy by Steve Yegge, you can find a lot more.

People are doing a lot of things to keep their code concise and prevent codebase from becoming too big, often even by sacrificing performance (where it doesn't matter much). So, do you really think that 100 lines of dead code can be a good thing?

Only place I have seen this being useful is in cases where you want to quickly disable something and test / backfill (Say program does steps A,B,C - all take a long time. During backfill, you might choose to disable B and C for time being to speed it up if you know they are not necessary). But the truth is that in all cases this is very hacky. And if you do see a long term usage for your backfill code, you should write your code in such a way that it uses config to pick a choice instead of using such hacks.
My quick rule-of-the-thumb is that never checkin this kind of code. If you need a checkin/version control, that means you will be able to get back to this sometime soon, and that is always a bad thing in light of changing requirements.

Actually, there is one way its useful to have "dead" code: when you want your program to throw a huge exception because it reached something that should never have happened. This would be either a compiler error, or if someone up the line changed the logic in the test you are using, and screwed it up. Either way, your "else" sends up fireworks. In this case, you'd want to ifdef it out for release.

It's vitally important that the error message state something like "Panic: we aren't ever supposed to be here at line %d in file %s"...