Code Smells, Switch Statement – Object Oriented Abusers: Day 7

[Javier Arroyo]

This article is Day 7 and first article of the Object Oriented Abusers category in a multi-post series about refactoring Code Smells. Code Smells are characteristics that can negatively impact code; ranging from hindered read-ability, clutter, unscale-ability, even outdated programming practices.

Refactoring examples will be mostly, if not all, from the Imperative/OO perspective as ServiceNow OOTB code is somewhat written as such. I will try to be brief and attempt to use language features specific to JavaScript that may differ from those used by classically trained coders.

What are Object Oriented Abusers:

Inaccurate or undeveloped OO principles. 

Previous Code Smell articles

Bloaters

Code Smells, Refactoring Comments– Bloaters: Day 1 Code Smells, Refactoring Long Functions– Bloaters: Day 2 Code Smells, Refactoring Long Parameter Lists– Bloaters: Day 3 Code Smells, Refactoring Data Clumps Including Parameter Lists– Bloaters: Day 4 Code Smells, Refactoring Long Script Includes– Bloaters: Day 5 Code Smells, Primitive Obsession– Bloaters: Day 6

Day 7 - Refactoring Switch Statements

A switch statement is a flow condition control rooted in procedural languages that is supported in OO. Its procedural based execution within OO is what causes it to be a code smell. 

Examples of abusive switches

• Uncontrolled growth (checks 5 cases or >)
• not isolated to a single script include
• cases don't fail-over to methods
• contain logic that should be separated into SIs or at least methods in a SI

Solution

Binary operations can remain a switch. If the conditions grow into 3 or more, think of using  polymorphism, strategy pattern, or decision table. In a more purely JS strategy, use a factory to build the object with the expected behavior.

Why Primitive Obsession smells

• addition of new values leads to extending conditions each and every time
• can grow out of control
• doesn't take advantage of OO features
• must maintain multiple unrelated locations
• added cluter as it grows
• limited scalability
• cant be tested atomically
• related behavior isn't logically structured/grouped

Example of a Switch Statements

Valid binary use

function toYesNo (aBooleanValue) {
    var result;

    switch (aBooleanValue) {
        case true: 
            result = 'Yes';
            break;

        case false: 
            result = 'No';
            break;

        default:
            result = 'No';

    }

    return result;
} 

In the example above there will always be two conditions: a boolean true or false. The result will also always be the same: the string Yes or No. Because switch uses strict equality (===), such approach will always return the correct value (Yes or No) regardless of what is passed in.

During such cases, even at times when there will be three choices (IE: Left, Right, Straight), maybe four (North, South, East, West); arguably five conditions that make a switch a fitting solution. When nothing can ever be added besides the given choices and their outcomes are deterministic (given the same input. the output will always be the same). It's quite fine to use it.

Innacurate or undeveloped OO principles Example

It is said to be inaccurate or undeveloped because it doesn't use OO provided solutions, or those solutions are used incorrectly.

//some code from a long method or SI

switch (SomeCar.getBrand()) {
  case 'Ford':
    //code here
    break;
  
  case 'GMC':
    //code here
    break;

  case 'Chevrolet':
    //code here
    break;

  default:
    //code here
};

//some more code from a long method or SI

The above example quickly shows that when a new brand is added, the switch statement will need to be modified. Each time the same action is needed the same solution has to be applied, the script then begins to grown uncontrollably with procedural practices that lead to brittle code. It can be argued that a quick fix would be to extract the switch from where it is an placed in a method of it's own. That solution, however, doesn't solve the issue at hand. SomeCar is logically the best place to express what it means to doSomething. This means that behavior is contained within a ScriptInclude and not spread out about other SIs, bringing about the issues that are identified by Code Smells.

Example Solution

The OO perspective of the procedural solution above is to replace the switch with polymorphism. 

1. Create SIs to account for the brands needed by the switch
2. add a method named the same doSomething
3. replace switch with some car

function Ford () {

    return {doSomething: doSomething, type: 'SomeCar'};

    function doSomething () {
        gs.debug("I am a Ford");
    }
}

function Chevrolet () {

    return {doSomething: doSomething, type: 'SomeCar'};

    function doSomething () {
        gs.debug("I am a Chevy");
    }
}

function GMC () {

    return {doSomething: doSomething, type: 'SomeCar'};

    function doSomething () {
        gs.debug("I am a GMC");
    }
}

Above are the three SIs that will replace the switch statement. In a complete OO solution, Ford, Chevrolet, and GMC would inherit from a parent to account for when the vehicle isn't one of those.

The old code is then replaced do

//some code from a long method or SI

SomeCar.doSomething();

//some more code from a long method or SI

In a more JS-centric solution a factory would assume various things about a car and build the resulting object:

function makeCarBrand(DesiredFunctionalitySI, brandName) {
    //do some stuff to build the object
    
    var newCar = Object.assign({}, 
                        SomeStuffBuiltInternally, 
                        BaseFunctionalityForCarsScriptInclude, 
                        DesiredFunctionalityScriptInclude);

    newCar.brand = brandName;

   return newCar;
}

from there, anytime a car brand is needed, it is built by specifying what functionality is needed through DesiredFunctionalitySI which is then added to a Base functionality through makeCardBrand.  This then closes the SI to changes and limits downstream changes to (as long as the public API is left intact) to the factory. It also adds the ability to build a car brand in terms of behavior (what can i do) rather than what I am.

 //Ford = makeCarBrand(SomeObject, 'Ford'); or a different factory to makeFord(someObject);

The goal of replacing Switch statements is to help authors build a more eloquent and principled based coding experience that are meant to decrease code maintenance, extend scalability, reduce code sprawls and duplication by necessity, etc. Of the pluses addressed by refactoring OO Abusers is the indirect alleviation of smells not even considered to be such.