No switch / if-else if on class invariants

If it is an invariant, why testing on it?

The typical code is

protected final int type;

void foo() {

switch (type) {

case 1:do1();break;

case 2:do2();break;

default:

}

}

This is poor man’s polymorphism. In fact a proper class model would allow to have an empty method foo and two overrides that contain the code of do1 and do2. Normally the member type becomes obsolete, too.

Arguably it seems to be more complex to have at least three classes instead of one. On the other hand each of these classes will be simpler, although the boiler-plate code for a class declaration will not go.

Anyway, you should pay this price. The shortcut above will grown into a big problem. Look at method bar()

void bar() {

switch (type) {

case 4:odd1();break;

case 2:odd2();break;

default: odd0();

}

}

Can you tell me why this method handles different cases and what you have to do to add a new case. Is it perhaps an error that foo doesn’t do anything in case 4 (which perhaps cameĀ  later)…

The class concept is there to bundle behavior into some that is self-sustaining to avoid this coding style above which is hard to maintain and virtually impossible to analyse. Perhaps type 3 is completly void, but this is hard to see when you have to analyse switches and else-ifs to find out that after reading 200 lines that your case is ignored.

Sometimes, when you already have a quite complex model, adding another normalization will multiply the effort. Besides looking at perhaps some behavioral pattern to simply the static model: Pay this price. The worst thing to have is a mixture of inheritance and the switches: Imagine doX() would have overrides and checked itself on other invariants… it won’t take long as the number of possible paths will challenge the the grow of Ackermann’s function. In this case it might had been better to stay with Pascal and the code above.

No instanceof

Methods should never reference to instanceof with

  • this: Definitely some shortcoming in the model
  • an object where they plan to call a method on

The remaining cases are mostly factories, we need them.

The (this instanceof Some.class) shows clearly that the program doesn’t know what it is doing, or at least this class has an identity crisis. it is so simple to implement something in Some that does what you need and let the runtime perform the instanceof and the complier verify that you handled all cases.

The “check before call” is mostly accompanied by a downcast, which is not nice but cannot be avoided always (see programming against interfaces). Really bad is:

void foo(Bar bar) {

if(bar instanceof WhiskeyBar) bar.buyDrink();

bar.locateRestroom();

}

This states a strategy. Your strategy is to get a whiskey - if it is a strategy, use one. This example is simple, but combined calls like this with many strategies will get you code that gets incomprehensible and non extensible. Externalize these decisions into a behavioral pattern.