April 18, 2014
Hot Topics:
RSS RSS feed Download our iPhone app

Working with Design Patterns: Template Method, Page 2

  • April 27, 2007
  • By Jeff Langr
  • Send Email »
  • More Articles »

The two rules, CheckDependentsRule and CheckStateRule, appear to have some similarities. Chunks of the eval method are the same, and a few supporting methods appear to be the same. We want to eliminate the redundancies between these two classes, which should reduce our risk and future maintenance costs.

Should we use delegation or inheritance? Normally I prefer to start by considering delegation, as it doesn't introduce as-strong dependencies. Delegation is ideal when we need to re-use common constructs and logic. But in this case, our two classes are behaviorally related. Both share the same goals and behaviors. We'll create a common class, BaseRule, and slowly pull common functionality up into it.

As the first step, we simply create an empty superclass and have both Rule classes extend it. We then, naturally, run our tests.

public class BaseRule {
}

public class CheckStateRule extends BaseRule implements Rule {
...

public class CheckDependentsRule extends BaseRule implements Rule {
...

If we look at CheckDependentsRule, the last chunk of code in the try block appears to replicate the functionality in CheckStateRule's switchReturnValues method.

         boolean returnVal1 = returnVal;
         if (!returnTrue)
            returnVal1 = !returnVal1;

         String retErrStr = null;
         if (returnVal1 == false) {
            if (returnTrue)
               retErrStr = "Dependent info incomplete";
            else
               retErrStr = "All dependent info provided";
         }

         List<Object> res = new ArrayList<Object>();
         res.add(new Boolean(returnVal1));
         res.add(retErrStr);
We extract this code to a separate method with the same name as in CheckStateRule:
         // return value
         List<Object> returns = switchReturnValues(returnTrue, returnVal,
               "Dependent info incomplete", "All dependent info provided");
         ...
   }

   private List<Object> switchReturnValues(boolean returnTrue, boolean returnVal, String trueMsg, String falseMsg) {
      boolean returnVal1 = returnVal;
      if (!returnTrue)
         returnVal1 = !returnVal1;

      String retErrStr = null;
      if (returnVal1 == false) {
         if (returnTrue)
            retErrStr = trueMsg;
         else
            retErrStr = falseMsg;
      }

      List<Object> res = new ArrayList<Object>();
      res.add(new Boolean(returnVal1));
      res.add(retErrStr);
      return res;
   }

At this point we can visually inspect the two switchReturnValues methods and see if they indeed contain the same logic. Since we have good unit tests, we can spend only a little bit of time on this comparision, enough to get a good warm fuzzy. The real answer comes by trying to eliminate the redundancy: We pull one of them up to the BaseRule superclass (run tests), and eliminate the other (run tests again). All remains green.

import java.util.*;

public class BaseRule {
   protected List<Object> switchReturnValues(boolean returnTrue,
         boolean returnVal, String trueMsg, String falseMsg) {
      boolean returnVal1 = returnVal;
      if (!returnTrue)
         returnVal1 = !returnVal1;

      String retErrStr = null;
      if (returnVal1 == false) {
         if (returnTrue)
            retErrStr = trueMsg;
         else
            retErrStr = falseMsg;
      }

      List<Object> res = new ArrayList<Object>();
      res.add(new Boolean(returnVal1));
      res.add(retErrStr);
      return res;
   }
}

Getting rid of duplication often involves making things look the same. In both rule classes, we have a hasMissingData flag. We also have a String field that appears to hold an error message, but its name is different. We rename the field in CheckStateRule from errStr to errorMessage (and run tests). We also make both fields private in CheckStateRule, since they are both private in CheckDependentsRule. We run our tests. We do the same thing with the hasMissingData/missingData fields.

Once we complete the preparatory work of making things look the same, we pull up both the fields and their accessor (getter) methods to BaseRule. We make the fields protected instead of private. This is perhaps a questionable design decision, but we can look to correct it once the rest of our current refactoring is complete. We verify that our tests still pass.

One more simple refactoring: both classes define an EXCEPT constant. We pull this definition to the superclass and eliminate from both subclasses.

Now the fun work begins. The eval method is still a bit long. But if we look at the structure of the eval method within the CheckStateRule, we note that it has the following policy:

  • extract arg values
  • extract form data
  • execute rule
  • prepare return values, negating if necessary
  • handle any trapped exception from the above three steps as a rule failure

CheckDependentsRule has pretty much the same structure, except that it doesn't extract any argument values. Starting with CheckStateRule, we break up the function into method calls that correspond to this policy.

import java.util.*;

/**
 *
 * Check if the state the case is located in is equal to argument passed in.
 */
public class CheckStateRule extends BaseRule implements Rule {
   private String argVal;
   private String formState;

   public boolean eval(Case c, List<String> val, boolean returnTrue) {
      try {
         return evalWithThrow(c, val, returnTrue);
      } catch (Exception e) {
         errorMessage = BaseRule.EXCEPT + e.getMessage();
         hasMissingData = true;
         return false;
      }
   }

   private boolean evalWithThrow(Case c, List<String> val, boolean returnTrue) {
      if (!extractArgs(val))
         return false;
      if (!extractFormData(c))
         return false;
      boolean returnVal = executeRule(c);
      return prepareReturnValues(returnTrue, returnVal);
   }

   private boolean extractFormData(Case c) {
      // get the location object off the form and get the state the
      // case resides in
      Location loc = (Location)c.getFormComposite("location");
      if (loc == null) {
         errorMessage = "Cannot determine the state this case is located in.";
         hasMissingData = true;
         return false;
      }
      String st = loc.getAttribute("state");
      formState = (st == null ? null : st.toUpperCase());
      if (formState == null) {
         errorMessage = "Cannot determine the state this case is located in.";
         hasMissingData = true;
         return false;
      }
      return true;
   }

   private boolean prepareReturnValues(boolean returnTrue, boolean returnVal) {
      List<Object> returns = switchReturnValues(returnTrue, returnVal,
            "Case does not reside in '" + argVal + "'", "Case resides in '"
                  + argVal + "'");
      returnVal = ((Boolean)returns.get(0)).booleanValue();
      errorMessage = (String)returns.get(1);
      return returnVal;
   }

   private boolean executeRule(Case c) {
      return formState.equals(argVal);
   }

   private boolean extractArgs(List<String> val) {
      if (val == null || val.size() != 1) {
         errorMessage = "Can't convert value list to a state";
         hasMissingData = true;
         return false;
      }
      argVal = (String)val.get(0);
      return true;
   }
}





Page 2 of 3



Comment and Contribute

 


(Maximum characters: 1200). You have characters left.

 

 


Sitemap | Contact Us

Rocket Fuel