How to explain why a developer should avoid case s

Question

I have a few times needed to try and explain to a developer why they should change a class which has multiple instances of

switch(m_type) {
  case TYPE_A:
    ...
    break;

  case TYPE_A:
    ...
    break

  ...
}

Where m_type is a (final) reference to an enum defining what type of instance this is.

Whenever I see this I think it's time to split the class out into multiple subclasses where the concrete class you create then defines the type. The argument I always get back is 'It didn't seem worth creating all these classes with hardly any code in, what are they going to have in there? like 2 methods or something!'

The best explanation I've seen is that unnecessary control logic should be avoided. If the code can be restructured to remove or avoid a branch, if, switch, etc. it should. If not, then use the control statement that is the most appropriate. From here but I'm not convinced this will be well received since the dev in question always thinks their code isn't too complex anyway so probably won't get why it matters.

This is a common enough mistake though with a very common attitude to why it exists, has anyone had any luck helping a developer understand the mistake they are making?

Thanks,

Robin

Answer 1

Tell him that he needs to use an extra variable to store the object type. Tell him about the vtable (if using C++) and explain how that idiom is baked into the language and thus idiomatic.

Answer 2

Well this is why object oriented programing was invented in the first place. Tell him with static binding, which a case statement is, any new type introduced needs old code to be rewritten, recompiled, retested and redistributed.

Whereas with classes and virtual functions new code can be called by old code.

Answer 3

Lead by example, show some places where you have removed duplicate switch statements.

Answer 4

You're asking for justification for the refactoring: Replace Conditional with Polymorphism, or the more specific: Replace Type Code with Subclasses. The Anti-If Campaign has some relevant articles.

The sourcemaking website says:

The advantage of Replace Type Code with Subclasses is that it moves knowledge of the variant behavior from clients of the class to the class itself. If I add new variants, all I need to do is add a subclass. Without polymorphism I have to find all the conditionals and change those. So this refactoring is particularly valuable when variants keep changing.

I don't think there's much to add to that.

Answer 5

The code you described is in direct violation of the Open/Closed Principle (the "O" in SOLID). With respect to OOP, that should be enough to make your case. ;-)

Answer 6

Make sure they write thorough unit tests for the code in question. Then explain that when a new 'type' is added in future and they need to change this code, they will now have broken unit tests that they need to fix.