Fixing the if smell

From time to time we might end up with some huge ‘if’ statements in our codebase. Those statements have to be maintained and change the same code block over and over again. This is common also in cases where the ‘if’ statement checks if a variable belongs in a certain range of values.

Supposing you have an enum

public enum FoodType {

    FRUIT,
    VEGETABLES,
    RED_MEAT,
    WHITE_MEAT,
    FISH,
    DIARY,
    CERIAL
}

And you have a function making some recommendations

    public String recommend(FoodType foodType) {

        if(foodType==FoodType.FISH||foodType==FoodType.RED_MEAT||foodType==FoodType.WHITE_MEAT) {

            //execute a procedure
        } else if(foodType==FoodType.FRUIT||foodType==FoodType.VEGETABLES) {
            //execute a procedure
        } else {
            //execute a procedure
        }
    }

Now as you can see, a decision is made. The decision has to do with certain types of food which happen to belong under a specific group.
Fish, red meat and white meat are good for a user who prefers protein for his meal while fruit and vegetables are suited more for a fiber based diet.
In future cases this enum might be enhanced and more food types added to it.
The ‘if’ code block will have to be changed. Also in case, this complex ‘if’ statement is used in other files you will have to alter each file.
Not only you will have a huge if block but also a block which has to be maintained on each file and this might be error prone.

In order to avoid that you can change the contents of the if statement into a function.

package com.gkatzioura;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import static com.gkatzioura.FoodType.*;

public class DietFilter {

    private static final Set FOODS_WITH_PROTEIN = Collections.unmodifiableSet(new HashSet(Arrays.asList(
            FISH,
            RED_MEAT,
            WHITE_MEAT)));

    private static final Set FOODS_WITH_FIBER = Collections.unmodifiableSet(new HashSet(Arrays.asList(
            FRUIT,
            VEGETABLES)));

    public static boolean proteinBased(FoodType foodType) {
        return FOODS_WITH_PROTEIN.contains(foodType);
    }

    public static boolean fiberBased(FoodType foodType) {
        return FOODS_WITH_FIBER.contains(foodType);
    }

}

So instead of adding each single case of food type, inside an ‘if’ statement we created a function which checks if the argument given belongs to a specific group.

Therefore your ‘if’ statement will change into this.

    public String recommend(FoodType foodType) {

        if(DietFilter.proteinBased(foodType)) {

            //execute a procedure
        } else if(DietFilter.fiberBased(foodType)) {
            //execute a procedure
        } else {
            //execute a procedure
        }
    }

If more food types are added to the enum, the developer will only have to change the construction of the set and add the extra food type.
It will be much easier than changing multiple parts of the code and it is way more readable.