Refactoring conditional logic

A couple of weeks back I was working on a piece of code that was becoming a tangled mess of if-else conditions. I was finding it difficult to reason and adding a new condition without breaking the existing code was becoming difficult. I knew as we will discover rules for new conditions the code in the class will grow. I was not satisfied with the code so started looking for ways to refactor the code. The code used in this post uses Java 8 as the programming language.

The code in discussion looked something like this.

public class CountryStateFilter {

      public boolean test(CustomObject obj) {
        Character countryInclExclFlag = obj.getCountryInclExclFlag();
        Character stateInclExclFlag = obj.getStateInclExclFlag();

        Predicate<String> countryPredicate = country -> Objects.equals(country, homeCountry);
        Predicate<String> statePredicate = state -> Objects.equals(state, homeState);

        if (Objects.equals('I', Character.toUpperCase(countryInclExclFlag))) {
            if (CharMatcher.whitespace().test(stateInclExclFlag)) { // I''
                return true;
            } else if (Objects.equals('I', stateInclExclFlag)) { // II
                return false;
            } else { // IE
                return true;
            }
        } else if (Objects.equals('E', Character.toUpperCase(countryInclExclFlag))) {
            if (CharMatcher.whitespace().test(stateInclExclFlag)) { // E''
                return true;
            } else if (Objects.equals('I', stateInclExclFlag)) { // EI
                return false;
            } else { // EE
                return true;
            }
        } else {
            if (CharMatcher.whitespace().test(stateInclExclFlag)) { // ''''
                return true;
            } else if (Objects.equals('I', stateInclExclFlag)) { // ''I
                return false;
            } else { // ''E
                return true;
            }
        }
    }
}

I added little comments to help me understand what value of countryInclExclFlag and stateInclExclFlag I was working with. The code above is just an example so I am directly returning a boolean value. In the real code, we are doing calculation and based on them we return the boolean value. The code above worked and met the desired objective.

In the next sprint I had to extend the above code to incorporate flag for city. I also learnt that in future we have to add support for zip flag as well. Adding support for city and zip flags in the code shown above looked error prone and tedious as with each flag conditional combinations will grow . This made me look for a better solution. I found a better solution in the book Refactoring to Patterns by Joshua Kerievsky.

Create a Command for each action. Store the Commands in a collection and replace the conditional logic with code to fetch and execute Commands.

This refactoring involves two main steps.

1. In the first step, you have to create Map that holds all the conditions by their corresponding key. The key in our case is the combination of flags like 'II', 'EE', ' I' . As shown below, we created a conditions Map .

   public class CountryStateFilter {
   
      private Map conditions;
   
      public CountryStateFilter(){
          this.conditions = new HashMap();
          this.conditions.put("I ", obj -> true);
          this.conditions.put("II", obj -> false);
          this.conditions.put("IE", obj -> true);
          this.conditions.put("E ", obj -> true);
          this.conditions.put("EI", obj -> false);
          this.conditions.put("EE", obj -> true);
          this.conditions.put("  ", obj -> true);
          this.conditions.put(" I", obj -> false);
          this.conditions.put(" E", obj -> true);
      }
      // Removed for brevity
   }

   In real world code, you will not make use of the obj to determine the value of boolean.

2. The next step involves replacing the If-else code with code that loops up the right condition by key and executes it as shown below.

   public class CountryStateFilter {
   
      private Map conditions;
   
      public CountryStateFilter(){
          this.conditions = new HashMap();
          this.conditions.put("I ", obj -> true);
          this.conditions.put("II", obj -> false);
          this.conditions.put("IE", obj -> true);
          this.conditions.put("E ", obj -> true);
          this.conditions.put("EI", obj -> false);
          this.conditions.put("EE", obj -> true);
          this.conditions.put("  ", obj -> true);
          this.conditions.put(" I", obj -> false);
          this.conditions.put(" E", obj -> true);
      }
   
      public boolean test(CustomObject obj) {
          Character countryInclExclFlag = obj.getCountryInclExclFlag();
          Character stateInclExclFlag = obj.getStateInclExclFlag();
          String key = String.format("%s%s", countryInclExclFlag, stateInclExclFlag);
          return this.conditions.get(key).test(obj)
      }
   }

Now, to support more flags I just need to add new conditions in the conditions Map without changing existing code. This refactoring technique make it easy to declare new handler and register it in the conditions map so that it can invoked at runtime to perform an action.

One thing that I assumed is before you started refactoring you had a working test suite. You can only be sure you didn’t break anything during refactoring if your working test suite remain in working condition after refactoring. After I did this refactoring, all my test cases were still green that gave me confidence I didn’t break anything.