Our team uses eXtreme programming practices to manage development on a mission critical system for a large retail chain. We have not adopted all the practices, but use the majority of the practices.
Here is a list of the practices we use:
Every morning at the standup meeting, the team lead will report on the nightly integration build.
This report includes the following metrics:
- Unit tests passed, failed, ignored
- Fit test passed, failed, ignored, exceptions
- Test coverage (should be more than 80%)
- Cyclomatic complexity (should be less than 10)
- Code instructions (should be less than a 100)
- FxCop rule validations
Each day, metrics are compared to the previous day, and the project manager tracks these metrics to get an overall feel for the health of the project.
The component where this example code was found is a business process web service in a Service Oriented application. The component had, on average, a Cyclomatic Complexity of 8, and this was constant over a couple of weeks. Then suddenly, the complexity of the component jumped to 17. See figure 1 showing the code metrics.
What is Cyclomatic Complexity?
The following good introduction was done by Samudra Gupta on Java Boutique. This metric was introduced by Thomas McCabe and measures the structural complexity of a method.
Suppose you've got a particular implementation class that's become huge in size or too complex to maintain. Or, you've got a single class acting as the control class for a whole business layer, but there's too much business logic embedded within that one class. Or suppose again that you've been handed a class containing too much code duplication. These situations are what are referred to as "complexity."
Learning how to use these three metrics may help steer you towards the correct re-factoring steps:
- Cyclomatic Complexity
- Response for Class
- Weighted methods per class
Cyclomatic Complexity (CC) = number of decision points + 1
Decision points are conditional statements such as
while etc. Consider the following example:
public void isValidSearchCriteria( SearchCriteria s )
if( s != null )
In the above example, CC=2+1=3 (one
if + one
else + 1).
Cyclomatic complexity has enormous impact on the testability and maintainability of code. As the example shows, if you want to test the
isValidSearchCriteria() method, you have to write three tests to verify it: one for the method itself, and two for the decision points within the method. Clearly, if the CC value increases, and there is an increasing number of decision points within any method, it means more effort to test the method.
The following table summarizes the impact of CC values in terms of testing and maintenance of the code:
Low risk program
Most complex and highly unstable method
Identifying Code Problems
Now, let's get back to our method with the moderate risk complexity. At this point, I would like to mention a few things you should think about when you look over the suspect method:
- Code Intent and clarity – How easy it is to understand and figure out what the method is supposed to do.
- Code Smells - Identify common design problems in object-oriented code.
Here is the method:
The first things I found when I looked at this method were the following code smells:
- Long Method - The longer the method, the harder it is to see what it's doing. At the moment, my personal preference is to have methods that are not longer than ten lines of code. If it exceeds ten lines, I'll rather refactor and break the method up.
- Duplicated Code.
- Comments - Should only be used to clarify "why" not "what". Comments can quickly become verbose and reduce code clarity.
- Dead Code - A variable, parameter, method, code fragment, class, etc. that is not used anywhere.
I was also very puzzled about what the method was doing. What verification was taking place here? The intent of what the code was doing wasn't clear. After a good couple of minutes, I figured out that the request passed from the UI to the business process web service had two parameters that needed to be checked.
The following rules where established:
- If both where null or an empty string, then throw a business exception.
- If both where populated, throw a business exception.
- If only the Customer ID is populated, then check if the parameter maximum length isn't violated.
- If only the Product Number is populated, then check if the parameter maximum length isn't violated.
- The last thing I found was that the method returned a boolean, but it was impossible to return a false value, so returning anything is redundant.
Solving the Problems Identified
To solve the problems with the suspect method, I've mainly used the extract method as a refactoring, and have applied some new features available in .NET 2.0 to reduce the need to check for null and empty string.
Here is the refactored code:
- The main
- Method to check both parameters are not null and not empty strings:
- Method to check both parameters aren't populated:
- Method to check the
CustomerID for field length and to pad the parameter to the correct length with zeroes:
- Method to check
ProductNumber for field length and pad the parameter to the correct length with zeroes:
Points of Interest
One of the first things you'll notice when you look at the refactored
ValidateRequest method, is the ease with which you can read and understand what the method is trying to achieve by reading just the method names. Writing self documenting and easily readable code isn't hard to do, it just requires dedication.
What you will find upon doing it is that:
- Other people will enjoy working with your code more
- It will make you look more professional since it seems you care about others touching the code
- It will help you write better code since it forces you to clearly define what you are doing
The next point to note is that all the methods are small and real easy to understand. Due to this, everyone on the project can isolate bugs and extend functionality easily.
Testability is also improved due to the reduced complexity. We might now have five methods in the refactored code compared to the one method before, but the highest complexity is now 4. This helps us to isolate each code path easier, and reduces the risk of untested code, which normally helps increase test code coverage on the component.
I think the benefits of using eXtreme Programming practices are really unmistakable. Probably, the most beneficial is the constant feedback and communication between everyone on the project. Other practices like Fit and Test-Driven Development, Refactoring, and Continuous Integration are paramount to keeping the quality of the code up to the highest standard.
Probably, if you look at this specific example, if we didn't have the feedback loop happening every day at our stand-up meeting with the code metrics report about the health of the code, problems like the one above would just fall through the cracks and technical debt would build up, until the code base has grown so big and fragile that nobody would ever want to change anything.
- Measuring the Complexity of OO Systems by Samudra Gupta.
- Refactoring: Improving the Design of Existing Code by Martin Fowler.
- Test Driven Development: By Example by Kent Beck.
- Fit for Developing Software: Framework for Integrated Tests by Rick Mugridge, Ward Cunningham.