hginit - #ifdefs ridiculous

Question

I was reading Joel Spolsky's mercurial introduction when it struck me:

"And now what they do is this: each new feature is in a big #ifdef block. So they can work in one single trunk, while customers never see the new code until it’s debugged, and frankly, that’s ridiculous."

Why is this so ridiculous anyway, isn't this, if nothing else, simply simpler to handle? It's nothing fancy but does the trick - at least if you are already "married" with subversion.

What is the downside? I kind of don't get the argument.

Answer 1

I think the answer is given in the subsequent sentence:

Keeping stable and dev code separate is precisely what source code control is supposed to let you do.

By using #ifdef blocks, you are emulating functionality of a source control system with a C preprocessor. It's the wrong tool for the job.

The downside is that you probably either end up duplicating a lot of code (think of whole function definitions once inside, once outside the #ifdef) or have unreadable code (think of the #ifdef enclosing single lines within a function - possibly repeatedly).

In the first variant you end up fixing bugs twice (which wouldn't be necessary if you could just merge bugfixes from the stable branch to the dev branch), whereas in the second the original goal of separating stable and dev branches was not really achieved.

Answer 2

Having maintained code that was a rat's nest of #ifdefs (although for different reasons), it is ridiculous; it makes code much more difficult to read and much more difficult to maintain, especially when testing against multiple conditions.

Answer 3

It assumes all new code can be written without touching existing code. Often the only way to do that is to repeat your self a lot. (a new method for one that is similar but not exactly the same for example).

One of the major advantages of distributed VCS is that encourages the team to work on their own feature branch and not be worried about your work conflicting with others. This strategy negates that advantage.

To me it seems ridiculous since they seem to be deliberately misusing the tools. If your going to make the switch surely its worth learning how to use them.

Answer 4

#ifdef'ed code is not ridiculous, but - sad and terrible. And it is (today) the diagnosis for the developer

Well, Joel selected in "Subversion Re-education" The Bad Way (tm) of argumentation - he created special, degenerated situation with specially selected users and use this use-case as a justification for the thesis "Subversion suxx"

Spaghetti-code suxx more, unmanageable and unreadable code is more dangerous, than spending some time on re-education of "Right Merge" in Subversion - "Merge often, morons!!!" Branching and bidirectional merge was possible and usable in Subversion <1.5, now (after introducing mergeinfo) merge become even easier and more comfortable. Small overhead of monitoring "interesting" trees and sync-merges (often!!!) is fair price for developer, which get in return compact, clean, debuggable code.

Answer 5

What he is describing is actually a FeatureToggle, and there can be good reasons for using that instead of feature branches. The issue is with using #ifdef. Without very good management they can easily become a big mess littered all around the code-base.

Consider the following:

1. When do you remove an #ifdef? - Are you certain there is no builds left which still excludes that feature? Also not for that one (well paying) client for who you make some special builds?
2. Who's responsible for removing the #ifdef? - The developer who made the feature? Are you sure he will remember 4 weeks down the line, to check that the switch is not excluded in any build and can safely be removed? Will he be confident that it is safe to remove the #ifdef? What about that other extremely urgent task, doesn't that take priority over removing a measly #ifdef?

My experience says that such #ifdefs will be laying around forever, littering.

Answer 6

From experience (on a large project in a 600,000+ line codebase, which was kept out of releases for 3 years using #ifdefs) my arguments against using them would be:

• They reduce the readability of the code - particularly as some IDEs insist on placing them at the start of the line ignoring the indentation level.
• They add complexity, both to the devs working on the segregated feature and to the devs trying to work around them.
• That additional complexity slows down productivity - in our case I'm convinced it added several months to the project.
• Removing them once the feature is released is not a zero cost task - in our case it was a week long task for someone to go through and remove all the #ifdefs and resolve the conflicts with the code that had been added in #elseifs.

All in all it was a glorious day when we migrated to Mercurial - we still have a huge, overly complex code base, but at least we can branch efficiently now.