How to Read and Improve the C.R.A.P Index of your code

When I first started using PHPUnit to test my code, I ran across a statistic I had never seen before: the C.R.A.P index. After doing a little research and asking the right questions, I was able to get a firm grasp on what the C.R.A.P index is and what it means for my code.

What is the C.R.A.P Index?

According to Alberto Savoia:

The C.R.A.P. (Change Risk Analysis and Predictions) index is designed to analyze and predict the amount of effort, pain, and time required to maintain an existing body of code.

Basically it’s a number designed to highlight potential problem areas in the code you’ve written. The higher the number, the crappier your code may be and the more likely it should be reexamined and refactored.

Testing This Theory – Write Some Crappy Code

In order to see just how the C.R.A.P index works, we must first write some crappy code. In the following snippet, I’ve created a PHP object called CrapClass that contains the method listCities:

<?php
class CrapClass {
    /**
     * Prints a short list of cities within a given state
     * @access public
     * @author Levi Hackwith
     * @param string $state The state whose cities we want to list
     * @return boolean Returns true if everything ran fine, or throws an
     * UnexpectedValueException
     */
    public function listCities($state){
        if($state == 'Nebraska') {
            print('Omaha, Lincoln, Bellevue, LaVista');
        } elseif ($state == 'Iowa') {
            print('Des Moines, Council Bluffs, Red Oak');
        } elseif ($state == 'Florida') {
            print('Tampa, Pensacola, Miami');
        } elseif($state == 'Massachusetts') {
            print('Acton, Andover, Bedford');
        } elseif($state == 'Alabama') {
            print('Abbeville, Adamsville, Addison');
        } else {
            throw new UnexpectedValueException("Unknown State: '$state'");
        }
        return TRUE;
    }
}
?>

Yeah, that’ll do nicely. Now let’s create a unit test that doesn’t really test any of the code:

<?php
require_once dirname(__FILE__) . '/../CrapClass.php';
class CrapClassNoTest extends PHPUnit_Framework_TestCase {

    /**
     * @var CrapClass
     */
    protected $crapClass;

    /**
     * Sets up the fixture, for example, opens a network connection.
     * This method is called before a test is executed.
     */
    protected function setUp() {
        $this->crapClass = new CrapClass;
    }

    /**
     * Tears down the fixture, for example, closes a network connection.
     * This method is called after a test is executed.
     */
    protected function tearDown() {

    }

    /**
     * @todo Implement testListCities().
     */
    public function testListCities() {
        // Remove the following lines when you implement this test.
        $this->markTestIncomplete('This test has not been implemented yet.');
    }

}
?>

When we run the code coverage report for this test case, this is what we get (click for a larger view):

Crap Index - No Tests

Wow, a C.R.A.P index of 42. Why so high? Well, in addition to the 6 possible logic paths inside of the listCities method, the amount of code-coverage provided by the unit test class is at zero percent. So what’s the first step in improving our code?

Write More Complete Unit Tests

Part of what makes up the C.R.A.P index is how much of your code is covered by unit tests. The more the code is covered, the lower the C.R.A.P index. Let’s see what happens when we add a few tests to our testing class:

<?php

require_once dirname(__FILE__) . '/../CrapClass.php';

/**
 * Test class for CrapClass.
 * Generated by PHPUnit on 2011-01-22 at 19:01:20.
 */
class CrapClasPartialTest extends PHPUnit_Extensions_OutputTestCase {

    /**
     * @var CrapClass
     */
    protected $crapClass;

    /**
     * Sets up the fixture, for example, opens a network connection.
     * This method is called before a test is executed.
     */
    protected function setUp() {
        $this->crapClass = new CrapClass();
    }

    /**
     * Tears down the fixture, for example, closes a network connection.
     * This method is called after a test is executed.
     */
    protected function tearDown() {

    }
    public function statesProvider() {
        return array(
            array('Nebraska'),
            array('Iowa')
        );
    }
    /**
     * @dataProvider statesProvider
     */
    public function testListCities($state) {
        switch($state) {
            case 'Nebraska':
                $this->expectOutputString(
                    'Omaha, Lincoln, Bellevue, LaVista'
                );
                break;
            case 'Iowa':
                $this->expectOutputString(
                    'Des Moines, Council Bluffs, Red Oak'
                );
                break;
        }
        $this->crapClass->listCities($state);
    }

}
?>

And here’s the new coverage report:

Partial Coverage

By adding a unit test that covers just two of the 6 possible paths in our listCities method, we’ve dropped the C.R.A.P index from 42 to 11.62, a marked improvement. Let’s see what happens when we write tests to cover all the code:

<?php

require_once dirname(__FILE__) . '/../CrapClass.php';

/**
 * Test class for CrapClass.
 * Generated by PHPUnit on 2011-01-22 at 19:01:20.
 */
class CrapClassCompleteTest extends PHPUnit_Extensions_OutputTestCase {

    /**
     * @var CrapClass
     */
    protected $crapClass;

    /**
     * Sets up the fixture, for example, opens a network connection.
     * This method is called before a test is executed.
     */
    protected function setUp() {
        $this->crapClass = new CrapClass();
    }

    /**
     * Tears down the fixture, for example, closes a network connection.
     * This method is called after a test is executed.
     */
    protected function tearDown() {

    }
    public function statesProvider() {
        return array(
            array('Nebraska'),
            array('Iowa'),
            array('Florida'),
            array('Massachusetts'),
            array('Alabama')
        );
    }
    /**
     * @dataProvider statesProvider
     */
    public function testListCities($state) {
        switch($state) {
            case 'Nebraska':
                $this->expectOutputString(
                    'Omaha, Lincoln, Bellevue, LaVista'
                );
                break;
            case 'Iowa':
                $this->expectOutputString(
                    'Des Moines, Council Bluffs, Red Oak'
                );
                break;
            case 'Florida':
                $this->expectOutputString(
                    'Tampa, Pensacola, Miami'
                );
                break;
            case 'Massachusetts':
                $this->expectOutputString(
                    'Acton, Andover, Bedford'
                );
                break;
            case 'Alabama':
                $this->expectOutputString(
                    'Abbeville, Adamsville, Addison'
                );
                break;
        }
        $this->crapClass->listCities($state);
    }
    /**
     * @expectedException UnexpectedValueException
     */
    public function testListCitiesException() {
        try {
            $this->crapClass->listCities('California');
        } catch(UnexpectedValueException $e) {
            throw $e;
        }
    }

}
?>

Complete Test

Just by writing more complete unit tests, our C.R.A.P index for the listCities method went from 11.62 to 6. While this is all well and good, the C.R.A.P index doesn’t just take into account code coverage, it also calculates code complexity. Which brings me to another method of lowering the C.R.A.P index of our code:

Write Less-Complex Code

The other factor used in calculating the C.R.A.P index, is the number of logic paths found within the code. The more individual paths found, the harder the code is to maintain and the higher the index will be. Before we begin refactoring, let’s take a closer look at our listCities method (I’ve numbered all the possible logic-paths):

<?php
class CrapClass {
    /**
     * Prints a short list of cities within a given state
     * @access public
     * @author Levi Hackwith
     * @param string $state The state whose cities we want to list
     * @return boolean Returns true if everything ran fine, or throws an
     *  UnexpectedValueException
     */
    public function listCities($state){
        if($state == 'Nebraska') { // 1
            print('Omaha, Lincoln, Bellevue, LaVista');
        } elseif ($state == 'Iowa') { // 2
            print('Des Moines, Council Bluffs, Red Oak');
        } elseif ($state == 'Florida') { // 3
            print('Tampa, Pensacola, Miami');
        } elseif($state == 'Massachusetts') { // 4
            print('Acton, Andover, Bedford');
        } elseif($state == 'Alabama') { // 5
            print('Abbeville, Adamsville, Addison');
        } else { // 6
            throw new UnexpectedValueException("Unknown State: '$state'");
        }
        return TRUE;
    }
}
?>

There are a total of six possible logic-paths in this method. Let’s reduce that number, rereun the unit tests, and see what happens:

<?php
class CrapClass {
    private $states = array();

    public function __construct() {
        $this->states = array(
            'Nebraska' => array('Omaha', 'Lincoln', 'Bellevue', 'LaVista'),
            'Iowa' => array('Des Moines', 'Council Bluffs', 'Red Oak'),
            'Florida' => array('Tampa', 'Pensacola', 'Miami'),
            'Massachusetts' => array('Acton', 'Andover', 'Bedford'),
            'Alabama' => array('Abbeville', 'Adamsville', 'Addison')
        );
        return TRUE;
    }
    /**
     * Prints a short list of cities within a given state
     * @access public
     * @author Levi Hackwith
     * @param string $state The state whose cities we want to list
     * @return boolean True if everything ran fine, or throws an
     *  UnexpectedValueException
     */
    public function listCities($state){
        if(isset($this->states[$state])) {
            echo implode(', ', $this->states[$state]);
            return TRUE;
        } else {
            throw new UnexpectedValueException("Unknown State: '$state'");
        }
    }
}
?>

Complete Coverage

By refactoring our code, we were able to lower our method’s C.R.A.P index to 2 and make the listCities method much more maintainable.

Final Thoughts

While lowering the C.R.A.P index of our code is always a good goal, it can easily create more problems than it solves. Refactoring your code just lower a number on a report is never a good idea. By arbitrarily lowering the C.R.A.P index, or any other programming metric for that matter, you not only go against the metric’s original intent, but you add an artificial and unnecessary complexity to your code that could end up doing more harm than good:

software metrics, in general, are just tools. No single metric can tell the whole story; it’s just one more data point. Metrics are meant to be used by developers, not the other way around – the metric should work for you, you should not have to work for the metric. Metrics should never be an end unto themselves. Metrics are meant to help you think, not to do the thinking for you. ~Alberto Savoia