I’ve been looking at a number of different tools and techniques that I can implement at work to help us figure out where this pile of code that our team has inherited is in the most trouble. One of the techniques that I’ve used in the past is to implement a cyclomatic complexity evaluation during each build. There are a few tools out there to do this, but, being a cheap bugger, I have only used the free devMetrics tool. I went out looking to grab it again this week for us to start implementing and lo-and-behold if the folks at Anticipating Minds haven’t thrown this great tool out to the Open Source world.
I downloaded the tool from SourceForge only to find that the only thing available is the source code, in a .NET 1.1 form, and not compilable. Disappointed is an understatement.
Like all other disappointments, I decided to have a couple of scotch to take the edge off. As I sat and reflected on the possible implementations of cyclomatic complexity I began to figure out that it really wasn’t what it was all cracked up to be. The standard calculation used to determine cyclomatic complexity is referred to as McCabe’s Metric and it comprises of a set of rules that essentially boil down to an increase in complexity by a value of 1 for each decision point that exists. Rough guidelines state that if your complexity number for a method is 1-5 then you’re good, 6-10 means you should consider some way to simplify and 11+ should be a big red flag for refactoring.
The purpose of these guidelines is to help keep your code at a place where, when reading individual methods, you have minimal decision points to consider at any one time. The way to reduce the complexity that exists in a specific method is to refactor the decision points out of that method and into other methods that are called by the original code. Take the following example where the first block of code has a complexity of 4 and the second block where the two methods both have a complexity of 2.
public double GetCustomerDiscountPercentage(Customer cust, DateTime applyDate)
{
if (cust.Type == CustomerType.Wholesale && applyDate >= START_OF_WINTER_DATE)
{
return 0.15;
}
else
{
return 0.10;
}
} > public double GetCustomerDiscountPercentage(Customer cust, DateTime applyDate)
{
if (WinterWholesaleCustomer(cust, applyDate) == true)
{
return 0.15;
}
else
{
return 0.10;
}
}
public bool WinterWholesaleCustomer(Customer cust, DateTime applyDate)
{
if (cust.Type == CustomerType.Wholesale && applyDate >= START_OF_WINTER_DATE)
{
return true;
}
else
{
return false;
}
}
By doing this refactoring (yes, it is quite trivial and the original was not all that hard to read) all we did was reallocate the total complexity. There is still an overall complexity of 4, but it has been split between two methods and has achieved the goal of reducing the level of complexity that you have to deal with at any one time. Or has it?
The refactoring that I’ve shown above has added an extra layer of call depth to your code, and for that I would argue that this should be treated as an addition of 1 to the cyclomatic complexity calculation in the GetCustomerDiscountPercentage method. Any time you add layers to the code’s execution path you force the person reading it to mentally manage a call stack. I don’t know about you, but I’m not capable of handling all that many items on my mental call stack.
What I’d like to see is a change to the criteria for complexity that is prescribed by McCabe’s Metric. In addition to having the complexity determination only using the keywords (or language specific equivalents) to if while repeat for and or , we should also add a component to the equation that deals with the depth of the code. Each time a method makes a call to another method, the calling method’s complexity should increase by a value of 1 for each level of depth that the call stack can reach because of that method call.
In my refactoring sample this would take the first block from it’s initial complexity of 4 to a refactored complexity of 3 which reflects the call to the newly created method.
I have other ideas for tweaking McCabe’s Metric, but I’m going to explore them more before I put them to virtual paper.