ONJava.com -- The Independent Source for Enterprise Java
oreilly.comSafari Books Online.Conferences.

advertisement

AddThis Social Bookmark Button

Code Improvement Through Cyclomatic Complexity
Pages: 1, 2

Case Study

Imagine a business-to-business application that handles orders for a restaurant chain. These orders can be for beverages, food, napkins, silverware, etc. Given a need for additional beverages, the system creates an XML document and transmits the document to a beverage vendor, which then fulfills the order. Different vendors have varying XML formats and, as the system is such a success for the chain's business, additional vendors have been joining the network over time. This places additional requirements on the system, which can increase complexity.



A common pattern of code that usually triggers elevated values of cyclomatic complexity is a large method with many if/else conditions that each do a small unit of work on some input or in creating the method's output. We've probably all crafted these methods and have probably discovered, at some later point in time, that the method has grown and continues to grow, as new requirements come up, triggering additional conditionals to handle them.

For example, the following snippet of code returns a List of XML documents depending on the orderType property found in the containing class.

if(MessageConstants.ORDER_TYPE_FOOD.equals(
   this.getOrderType())) {

   messages.add(new StandardMessage(
     this.getVendorId(), 
     this.getFoodOrderAcceptResponse()));

}else if(MessageConstants.ORDER_TYPE_BEVERAGE.
    equals(this.getOrderType())) {

    messages.add(new StandardMessage(
      this.getVendorId(), 
      this.getBeverageOrderAcceptResponse()));
}
//many more to follow...

As you can imagine, as new types of Orders are added to the system, the conditional statements grow to handle the resulting new XML documents. Not surprisingly, running PMD against this code produced a report indicating that this method, processRequest(), contained an elevated cyclomatic complexity.

The first step in stabilizing the code indeed proves to be somewhat arduous, as in order to craft an effective JUnit test suite, we have to craft multiple XML documents. What's more, the original class containing the processRequest() method is part of an inheritance hierarchy, which contains state and depends on other classes and their respective states.

Looking deeper into the code reveals the base class's constructor obtains an instance of a PropertyUtility, which happens to require that System variables are set to facilitate finding a property file!

We finally develop a suite of test cases that exercise our class to a level of confidence. Below is an example test case.

public void testHandleResponseGeneration
    throws Exception{

    String baseXML = //...obtain XML

    StandardMessageGenerator generator = 
        new StandardMessageGenerator(baseXML);

    List messages = generator.processRequest();

    Message mess = (Message)messages.get(0);

    TestCase.assertNotNull("message was null", 
        mess);
}

Once we've attained a level of confidence in the code, we can move on to other tasks in other objects, or further refine this one. As we're aware that this code is one of the core components of the application that is continually pushed and morphed by various developers into a high level of chaos, we decide to refactor.

One of the most effective techniques for breaking up a large, complex group of conditionals utilizes polymorphism. Based upon Replace Conditional with Polymorphism and a liberal interpretation of Design Pattern's Template Pattern, Factory Method Pattern, and Chain of Responsibility, we can craft an elegant framework that pushes each conditional into a separate class. With this surgical maneuver, we can now easily isolate each conditional, which allows us to write effective unit tests.

As the original code flow went down the conditional stack adding XML documents to a List if a condition held true, we will create an API that takes an Order object and a List object, as shown below. The Order object embodies all of the properties of the original class that forced the previous code to reference this.

public abstract 
   class AbstractResponseMessageHandlerBase {
  
  public AbstractResponseMessageHandlerBase() {
    super();        
  }
   
  abstract public void handleResponseGeneration(
     final OrderMessageInfoVO orderMessage, 
     List messageList) 
	 throws ResponseMessageException;
}

Each conditional will now embody a separate class, such as BeverageOrderAcceptResponseHandler, which matches a conditional testing whether the order is of type BEVERAGE.

With the abstract class defined, we simply push the corresponding conditional's logic into the template method. Below is BeverageOrderAcceptResponseHandler's implementation.

public void handleResponseGeneration(
  final OrderMessageInfoVO orderMessage, 
  List messageList) 
   throws ResponseMessageException {        
   
   try{
     if (MessageConstants.ORDER_TYPE_BEVERAGE.
       equals(orderMessage.getOrderType()) 
          && (MessageConstants.
          ORDER_ACTION_ORIGINAL.equals(
          orderMessage.getOrderAction()))) {
	            
      final String orderXML = 
          ResponseContentDelegate.
            getDefaultOrderResponseContent(
             orderMessage);
	            
      messageList.add(new 
         StandardMessage(orderMessage.getVendorId(), 
         orderXML));
	
     }
   }catch(MessageFormatException  e1){
       throw new ResponseMessageException(e1);
  }
}

We also define an Abstract Factory, IResponseHandlerAbstractFactory, which takes the Order object and returns a List; furthermore, we create an implementation of the factory.

Bearing in mind our liberal interpretation of various design patterns, we simulate a chain by adding all our ResponseHandler objects into a Collection and then iterating over the collection in the manufacture method, calling handleResponseGeneration and passing in our List and Order objects.

public class DefaultResponseHandlerFactory 
  implements IResponseHandlerAbstractFactory {

  final private static 
    AbstractResponseMessageHandlerBase[] HANDLERS;

  static{

    Collection temp = new ArrayList( );
    temp.add(new FoodOrderAcceptResponseHandler());
    temp.add(
       new BeverageOrderAcceptResponseHandler());
    temp.add(
       new NapkinOrderAcceptResponseHandler());
    temp.add(new CancelOrderReponseHandler());

    final AbstractResponseMessageHandlerBase[] 
      tmAr = new AbstractResponseMessageHandlerBase[
        temp.size()];

   HANDLERS = 
     (AbstractResponseMessageHandlerBase[])temp.
        toArray(tmAr);

 }

 public List manufacture(
   final OrderMessageInfoVO orderMessage) { 

  List messages = new ArrayList();

  for(int x = 0; x < HANDLERS.length; x++){
    AbstractResponseMessageHandlerBase iHandler = 
       HANDLERS[x];

     try{
       iHandler.handleResponseGeneration(
          orderMessage, messages);

     }catch(ResponseMessageException e1){
       Log.logError(e1);
     }
   }
 return messages;
 }
}

Our resulting framework is actually quite simple, as shown below in Figure 1. Our DefualtResponseHandlerFactory contains a Collection of AbstractResponseMessageHandlers. As more vendors come online, we simply extend AbstractResponseMessageHandler and ensure the newly created class is part of DefualtResponseHandlerFactory's collection.

Figure 1
Figure 1. Class diagram of refactored code.

Testing each ResponseHandler is now trivial. We craft a desired Order object and pass it into the corresponding ResponseHandler; subsequently, we ensure that the resulting List contains our desired XML document(s).

public void testHandleResponseGeneration 
  throws Exception{

  List messages = new ArrayList();

  OrderMessageInfoVO vo = //...obtain sample VO

  AbstractResponseMessageHandlerBase 
    handler = //...obtain desired Handler

  handler.handleResponseGeneration(vo, messages);

  Message mess = (Message)messages.get(0);

  TestCase.assertEquals("vendor id should be 7908", 
    "7908", mess.getUserId());
}

Additionally, after we have test cases for each handler, we then move on to test the DefaultResponseHandlerFactory. Again, testing this object could not be easier, as we essentially perform the same steps as above. We create an Order object, pass it into the factory, and ensure that the desired number of XML documents can be confirmed in the returned List. What's more, the original test cases we crafted before refactoring serve as regression tests!

Lessons Learned

Clearly, a metric like cyclomatic complexity can facilitate the discovery of potentially dangerous code. Its simple and logical representation of complexity has shown, time and time again, to correlate directly to defects. Indeed, running code-metric tools against familiar code bases will often support your deepest suspicions on difficult and unmanageable code.

As you utilize this metric and begin a process of stringent unit testing and refactoring, you may start to see the value in Test-Driven Development. As adopters of this revolutionary methodology can confirm, test-case development before actually writing implementation code yields highly elegant and, indeed, simple code, which is quite easy to test. In fact, projects that carry out Test-Driven Development will find their code bases are rarely flagged for high cyclomatic complexity values. Give it a try -- it will save you time down the road!

Andrew Glover is the founder and CTO of Vanward Technologies, a company specializing in building automated testing frameworks.


Return to ONJava.com.