Does anyone disagree with the statement: “using sw

Question

I have seen it written in multiple threads/comments on stackoverflow that using switch is just bad OOP style. Personally I disagree with this.

There will be many cases where you cannot add code (i.e. methods) to enum classes you want to switch on because you don't control them, perhaps they are in a 3rd-party jar file. There will be other cases where putting the functionality on the enum itself is a bad idea because it violates some separation-of-concerns considerations, or it is actually a function of something else as well as the enumeration.

Last of all, switches are concise and clear:

boolean investable;
switch (customer.getCategory()) {
    case SUB_PRIME:
    case MID_PRIME:
        investible = customer.getSavingsAccount().getBalance() > 1e6; break;
    case PRIME:
        investible = customer.isCeo(); break;
}

I'm not defending every use of switch and I'm not saying it's always the way to go. But statements like "Switch is a code smell" are just wrong, in my opinion. Does anyone else agree?

Answer 1

Taking your followup:

What if this is just the "investibility" logic for a customer wishing for a business loan. Perhaps the innvestibility decision of a customer for another product is really quite different ... Also, what if there are new products coming out all the time, each with different investibility decisions and I don't want to be updating my core Customer class every time this happens?

and one of your comments:

I'm not entirely sure about holding logic close to the data on which it operates. The real world doesn't work like this. When I ask for a loan, the bank decides whether I qualify. They don't ask me to decide for myself.

you are right, as far as this goes.

boolean investable = customer.isInvestable();

is not the optimal solution for the flexibility you're talking about. However, the original question didn't mention the existence of a separate Product base class.

Given the additional information now available, the best solution would appear to be

boolean investable = product.isInvestable(customer);

The investability decision is made (polymorphically!) by the Product, in accordance with your "real world" argument and it also avoids having to make new customer subclasses each time you add a product. The Product can use whatever methods it wants to make that determination based on the Customer's public interface. I'd still question whether there are appropriate additions which could be made to Customer's interface to eliminate the need for switch, but it may still be the least of all evils.

In the particular example provided, though, I'd be tempted to do something like:

if (customer.getCategory() < PRIME) {
    investable = customer.getSavingsAccount().getBalance() > 1e6;
} else {
    investable = customer.isCeo();
}

I find this cleaner and clearer than listing off every possible category in a switch, I suspect it's more likely to reflect the "real world" thought processes ("are they below prime?" vs. "are they sub-prime or mid-prime?"), and it avoids having to revisit this code if a SUPER_PRIME designation is added at some point.

Answer 2

Î know where you are coming from. Some languages force you to do this.

String str = getStr();
switch(str) {
case "POST" : this.doPost(); break;
case "GET" : this.doGet(); break;
//and the other http instructions
}

And now what? Sure, there is a nice OOP way to do it:

str.request(this);

Too bad that String cannot be extended and now you're considering writing a HttpInstruction class with 8 subclasses for each HttpInstruction. Honestly, especially when talking about parsers, it is just substantially difficult.

It isn't good OOP, that for sure, but good code is not always ... possible.

Let me disgress for a moment. I'm writing my thesis. I personally don't like the usual setup of recursive functions. You normally have like funcRec(arg1,arg), and func(arg1):=func(funcRec(arg1,0));

so I defined it in my thesis with default arguments. Not everybody knows the concept of a default argument. My thesis uses pseudo-code, but the professor had me change the algorithm to the traditional way, because you don't come across default arguments very often, so don't use them. Don't surprise your reader unnecessarily. I think he's right.

But the result is that now I'm stuck with a function whose sole purpose is to ship around default arguments - which could be prettier.

So, the bottom line is: Truely beautiful programs require excellent libraries, excellent code browsers and tools, bugtrackers of the qualitiy of FogBugz, at least, better more integrated, version management of git quality, and so forth. And, umm, people around you who can use all these things and know how to handle all of these things. And most of all: a beaauuuuutiful language that allows elegant solutions to tricky problems.

So, chances are, you're stuck with Java which makes it difficult to come up with a good replacement for switches in all situations. Self would have an elegant solution. But you're not using Self, and if you were, your colleagues wouldn't be able to read it, so forget that.

And now find a compromise.

It's sad, I know.

Answer 3

There are cases when you need to make a decision based on several options and polymorphism is overkill (YAGNI). In this case, switch is fine. Switch is just a tool and can be used or abused just as easily as any other tool.

It depends on what you're trying to do. The point is, however, that you should think twice when using switch as it may be an indication of a bad design.

Answer 4

I think whether a switch statement is poor OOP practice depends on where you are using the switch statement.

In a factory method for instance, it might be a very simple alternative to writing complicated and potentially buggy reflection-based code.

In most circumstances though, I think switches are just simply design. Quite often hiding operational complexity in different objects with the same method leads to more understandable and possibly even faster code. For instance, if you have a switch that executes lot then having pre-packaged objects could actually save some CPU cycles.

Answer 5

First, your goal should not be to achieve "good OO style" but good code. And "good" means at least correct, clear, readable and as simple as possible.

So I will reformulate the quesion to: "Is using switch a sign of bad code?", because that's really what I care about. Now I'll proceed to answer it.

Hm, that's a good question :) Usually, using a switch once is not a sign of bad code. But if you switch on the same thing at several points in your class, it's good to consider an alternative design in which you represent the switch alternatives with child classes - and when you consider that, ask yourself especially if classes created like that would be a specialization of the current class and would have an is-a relationship. If so, this gives more points to using inheritance.

One last comment: "Using [language feature X] at all is bad" is dangerously close to "The language designers were stupid to include [language feature X] in it".

Answer 6

If anything, I'm fed up with people describing this style of programming - in which a bunch of getters are added to the "low hanging" types (Customer, Account, Bank) and the useful code is sprayed around the system in "controllers", "helpers" and "utility" classes - as object orientated. Code like this is a smell in an OO system, and you should be asking why instead of getting offended.