Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Does anyone disagree with the statement: "using switch is bad OOP style"?

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?

like image 468
oxbow_lakes Avatar asked Feb 15 '09 14:02

oxbow_lakes


People also ask

Is using switch statement bad?

Switch case is not a bad syntax, but its usage in some cases categorizes it under code smell. It is considered a smell, if it is being used in OOPS. Thus, Switch case should be used very carefully.

Does anyone use switch statement?

Every developer tends to have a strong opinion about switch statements. Some love them and use them all the time, while others never even bother messing around with them.

Are switch statements good practice?

Even though using if-else statements to accomplish this logic results in a functional code, it is best practice to use a switch statement if you have multiple if-else. It makes your code more concise and clearer to understand.

What is a good way to avoid switch statements?

Here are some alternatives to switch statement : lookup table. polymorphism. pattern matching (especially used in functional programming, C++ templates)


2 Answers

I think statements like

Using a switch statement is bad OOP style.

and

Case statements can almost always be replaced with polymorphism.

are oversimplifying. The truth is that case statements that are switching on type are bad OOP style. These are the ones you want to replace with polymorphism. Switching on a value is fine.

like image 113
Bill the Lizard Avatar answered Sep 28 '22 05:09

Bill the Lizard


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.

like image 21
Dave Sherohman Avatar answered Sep 28 '22 04:09

Dave Sherohman