In the .NET code base of a product, there are #if pre-processor statements to de

Question

In the .NET code base of a product, there are #if pre-processor statements to define whether certain functionality should be available. Like so:

shared logic

#if version1
    some logic
#endif
#if version2
    some other logic
#endif
Personally I twitch at this structuring. I find it harder to read, and it makes me nervous for some reason.
I was told that the reasoning behind having it like this is that there are too many changes that happen in all of the versions. If the versions were stored in separate repositories or some other method, then any future changes would have to be implemented several times over.

I find the "too much to change" argument a bit vague, but I'll admit my experience with this kind of design is limited.

Is this really the best practice or the most common approach when it comes to handling multiple product versions with differing functionality? Is there a better alternative?

Explanation / Answer

I'd avoid having separate code bases. All that branching and merging creates lots of unnecessary maintenance work. Even worse when they get out of sync it becomes hard to determine which differences were intentional and which by mistake.

I prefer replacing #if preprocessor directives by ordinary if statements, that way all variants of the code need to compile and refactoring tools see all the code.

Beyond this simple general change you should look into refactoring the code, but how to do that depends on the nature of the differences.

Some suggestions:

Don't mix different variants within a single method. For example you could replace your code with:

SharedPart();
if(cond1)
Variant1();
if(cond2)
Variant2();
Use delegates, interfaces or virtual methods to dispatch to the different variants instead of if statements.

Inject* an interface containing the configuration instead of using global state. This allows you to write unit tests for all variants without recompiling or restarting.
For features that are either enabled or disabled, I'd create a Feature enum, together with an injected* interface that determines if the feature is available. The interface could look something like:

interface IGateKeeper
{
bool IsEnabled(Feature feature);
void Require(Feature feature);
}
where Require checks if the feature is enabled and throws a meaningful exception otherwise.

* If you're not familiar with dependency injection (DI) and inversion of control (IoC) learn about them.

Related Questions

Navigate

• Browse (All)
• Browse I
• Subjects

Integrity-first tutoring: explanations and feedback only — we do not complete graded work. Learn more.