Search code examples
phpunit-testingphpunitcode-coveragecyclomatic-complexity

How to enhance C.R.A.P. index for a switch-like function?


I have a very typical switch-like function that returns a clasiffication for a given input value (Body Mass Index in this case). (I'm working with this function, but it could be any other of the same nature)

For now, it's pretty much like so:

// ...

const TYPE_SEVERE_THINNESS = -3;
const TYPE_MODERATE_THINNESS = -2;
const TYPE_MILD_THINNESS = -1;
const TYPE_REGULAR = 0;
const TYPE_OVERWEIGHT = 1;
const TYPE_PRE_OBESE = 2;
const TYPE_OBESE_GRADE_I = 3;
const TYPE_OBESE_GRADE_II = 4;
const TYPE_OBESE_GRADE_III = 5;

// ...

public static function classification(float $bmi) : int
{
    if ($bmi <= 16.00) {
        return self::TYPE_SEVERE_THINNESS;
    }

    if ($bmi <= 16.99) {
        return self::TYPE_MODERATE_THINNESS;
    }

    if ($bmi <= 18.49) {
        return self::TYPE_MILD_THINNESS;
    }

    if ($bmi <= 24.99) {
        return self::TYPE_REGULAR;
    }

    if ($bmi <= 27.49) {
        return self::TYPE_OVERWEIGHT;
    }

    if ($bmi <= 29.99) {
        return self::TYPE_PRE_OBESE;
    }

    if ($bmi <= 34.99) {
        return self::TYPE_OBESE_GRADE_I;
    }

    if ($bmi <= 39.99) {
        return self::TYPE_OBESE_GRADE_II;
    }

    if ($bmi >= 40) {
        return self::TYPE_OBESE_GRADE_III;
    }
}

I'm going into a refactor round and I'm thinking of all possible enhancements to this function, specially for lowering the C.R.A.P. index (Change Risk Anti-Patterns) that at this moment is returning a value of 110.00.

Of course, there may be many possible enhancements. Feel free to suggest.

But my question specifically is about reducing cycolmatic complexity,

a) Is there any other way to structure this code so that C.R.A.P. index goes lower? b) In order to properly test this function, should I generate one test that asserts each case, or perform many tests to address each possible case? (I now the answer here could be "It's up to you", but maybe there exists a better way to reduce cyclomatic complexity and thus giving place to fewer tests that still cover all or most possible scenarios.)

If I had to match equal values, I'd just use a hashmap (key-value array), but since I'm evaluating ranges, the approach might be different.

Update: After building a test case with examples of each scenario, CRAP index went down to 10.01. Still, I believe there exist another way to perform the value lookup.

/**
 * Test it returns a valid WHO classification for BMI type
 *
 * @return void
 */
public function test_it_returns_a_valid_who_classification_for_bmi_type()
{
    // Sample bmi => expected type
    // Key must be a string later converted to float
    $testMatrix = [
        "15" => BMILevel::TYPE_SEVERE_THINNESS,
        "16.5" => BMILevel::TYPE_MODERATE_THINNESS,
        "18" => BMILevel::TYPE_MILD_THINNESS,
        "24" => BMILevel::TYPE_REGULAR,
        "27" => BMILevel::TYPE_OVERWEIGHT,
        "29" => BMILevel::TYPE_PRE_OBESE,
        "34" => BMILevel::TYPE_OBESE_GRADE_I,
        "39" => BMILevel::TYPE_OBESE_GRADE_II,
        "41" => BMILevel::TYPE_OBESE_GRADE_III,
    ];

    foreach ($testMatrix as $bmi => $categoryCheck) {
        $type = BMILevel::classification(floatval($bmi));

        $this->assertEquals($type, $categoryCheck);
    }
}

Solution

  • Ok, I managed to get a pretty reasonable C.R.A.P. index after some refactor while keeping tests green.

    I turned the function into a lookup with upper bound limits (bottom-up). I needed to add an extra case for out-of-range values and cover that case.

    Code:

    public static function classification(float $bmi) : int
    {
        $classifications = [
            ['limit' => 16.0 , 'type' => self::TYPE_SEVERE_THINNESS],
            ['limit' => 16.99, 'type' => self::TYPE_MODERATE_THINNESS],
            ['limit' => 18.49, 'type' => self::TYPE_MILD_THINNESS],
            ['limit' => 24.99, 'type' => self::TYPE_REGULAR],
            ['limit' => 27.49, 'type' => self::TYPE_OVERWEIGHT],
            ['limit' => 29.99, 'type' => self::TYPE_PRE_OBESE],
            ['limit' => 34.99, 'type' => self::TYPE_OBESE_GRADE_I],
            ['limit' => 39.99, 'type' => self::TYPE_OBESE_GRADE_II],
            ['limit' => 60   , 'type' => self::TYPE_OBESE_GRADE_III],
        ];
    
        foreach ($classifications as $classification) {
            if ($bmi <= $classification['limit']) {
                return $classification['type'];
            }
        }
    
        return self::TYPE_OBESE_GRADE_III;
    }
    

    Tests:

    /**
     * Test it returns a valid WHO classification for BMI type
     *
     * @return void
     */
    public function test_it_returns_a_valid_who_classification_for_bmi_type()
    {
        // Sample bmi => expected type
        // Key must be a string later converted to float
        $testMatrix = [
            "15" => BMILevel::TYPE_SEVERE_THINNESS,
            "16.5" => BMILevel::TYPE_MODERATE_THINNESS,
            "18" => BMILevel::TYPE_MILD_THINNESS,
            "24" => BMILevel::TYPE_REGULAR,
            "27" => BMILevel::TYPE_OVERWEIGHT,
            "29" => BMILevel::TYPE_PRE_OBESE,
            "34" => BMILevel::TYPE_OBESE_GRADE_I,
            "39" => BMILevel::TYPE_OBESE_GRADE_II,
            "41" => BMILevel::TYPE_OBESE_GRADE_III,
            "100" => BMILevel::TYPE_OBESE_GRADE_III, // After upper bound limit
        ];
    
        foreach ($testMatrix as $bmi => $categoryCheck) {
            $type = BMILevel::classification(floatval($bmi));
    
            $this->assertEquals($type, $categoryCheck);
        }
    }
    

    Hints on how to enhance the function are still welcome.