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.
Published on Java Code Geeks with permission by Emmanouil Gkatziouras, partner at our JCG program. See the original article here: Fixing the if smell Opinions expressed by Java Code Geeks contributors are their own. |
&& or || ??
You are right. In the original blogpost is ||. Unfortunately I cannot edit this one here.
From the code, its only || and the context they’ve provided will also have only || scenarios, so it makes sense in this particular case. simple utility methods also might work with if cases as is
It was fixed
This is java; you get super awesome enums. Why not put two booleans on the enum for protein and fiber. That way, any new values added to the enum HAVE to answer the fiber/protein question.