Sane API design (AKA C's #ifdef hell)

[H. S. Teoh]

In D circles we often talk about API design, and D lends itself quite
well to proper API design.  But you still have to apply it correctly.

Just today, while investigating a bug at a work project, I discovered
this nasty piece of work:

```c
	// file1.c:
	int getMaxMacGuffin() { ... }

	int getMaxMacGuffinNewPlatform() {
		...
		int n = getMaxMacGuffin();
		... // do something else with n
		return n;
	}

	// file2.c:
	int myFunc1() {
		...
		#ifdef BUILDING_FOR_NEW_PLATFORM
		int max = getMaxMacGuffinNewPlatform();
		#else
		int max = getMaxMacGuffin();
		#endif
		...
	}

	// file3.c:
	int myFunc2() {
		...
		#ifdef BUILDING_FOR_NEW_PLATFORM
		int max = getMaxMacGuffinNewPlatform();
		#else
		int max = getMaxMacGuffin();
		#endif
		...
	}

	// ... and so on, for like 4-5 different files in different
	// unrelated functions.
```

The bug was that under some circumstances the max number of MacGuffins
displayed doesn't match the actual max number.  Raise your hands, anyone
who guessed what the problem is.

...

Yeah you guessed it, in some obscure corner somewhere, somebody called
getMaxMacGuffin() without the accompanying #ifdef hell, thus obtaining
the wrong value.

The elephant in the room is, WHY ARE THERE TWO DIFFERENT FUNCTIONS FOR
COMPUTING THE MAX NUMBER OF MACGUFFINS?!?!?!

A bit of digging reveals that getMaxMacGuffin was the original function,
and getMaxMacGuffinNewPlatform was added later.  Apparently, whoever
implemented the latter went a little overboard with the philosophy of
"don't touch the original code, just code around it".  The chosen
solution, however, is totally insane.  Why would you want to sprinkle your
#ifdef hell all over the project everywhere getMaxMacGuffin is called?!

The much saner solution is:

```c
// renamed from the original getMaxMacGuffin()
static int _getMaxMacGuffin() { ... }

static int getMaxMacGuffinNewPlatform() {
		...
		int n = _getMaxMacGuffin();
		... // do something else with n
		return n;
}

// New version of the function, containing the ONLY actually necessary
// #ifdef:
int getMaxMacGuffin() {
	#ifdef BUILDING_FOR_NEW_PLATFORM
		return getMaxMacGuffinNewPlatform();
	#else
		return _getMaxMacGuffin();
	#endif
}

// Then everywhere else, just always call getMaxMacGuffin and delete all
// the rest of the #ifdef hell
```

After this refactoring, I didn't even need to know where the bug was; it
automatically fixed itself because now getMaxMacGuffin will always do
the right thing.

//

Thankfully, in D we don't have #ifdef hell...

... or do we?  Although the example I encountered today was in C, one
can easily conceive of the D equivalent:

```d
	// file1.c:
	int getMaxMacGuffin() { ... }

	int getMaxMacGuffinNewPlatform() {
		...
		auto n = getMaxMacGuffin();
		... // do something else with n
		return n;
	}

	// file1.c:
	int myFunc1() {
		...
		version(NewPlatform)
			auto max = getMaxMacGuffinNewPlatform();
		else
			auto max = getMaxMacGuffin();
		...
	}

	... // and so on
```

So we don't have #ifdef hell in D, but we *do* have version hell here,
and we'd end up in exactly the same situation as in the C version.  The
solution, of course, is also the same: move the version block inside the
function, and if needed move the original function body into a helper
function.

Thankfully, among D circles we generally hate boilerplate and like to
use metaprogramming to reduce or eliminate it. Still, used wrongly, even
D's better constructs can still lead to versioning hell, like above.


T

-- 
Frank disagreement binds closer than feigned agreement.