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

Question

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);
    }
}

score 1 · Answer 1 · answered Aug 20 '18 at 21:55

Tests normally allow you to reflect existing (and perhaps outdated) implementation:

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;

so what you go here is an ordered list (from top to bottom):

const TYPES = [-3, -2, -1, 0, 1, 2, 3, 4, 5];

or perhaps:

const TYPES = [1, 2, 3, 4, 5, 6, 7, 8, 9];
const TYPE_REFERENCE = 4;

and the corresponding values (as not being a key, not an issue w/ conversion):

const VALUES = [15, 16.5, 18, 24, 27, 29, 34, 39, 41];

and you might want to provide labels:

const LABELS = ["Severe Thinness", "Moderate Thinness", "Mild Thinness", 
                "Regular", "Overweight", "Pre-Obese", "Obese Grade I",
                "Obese Grade II", "Obese Grade III"];

So what is easily to imagine this most of all does not belong into code but data to configure the code with. The unit test then could test against different data-sets which will not only increase the stability of the system under test but also test which extensions (changes) are easily to apply w/ existing code.

Using data-providers in tests often make visible that the code-base itself most likely should have some data providers and not so much hard-encoding.

Thanks for the clarification!! – alariva Aug 20 '18 at 23:52 — alariva, Aug 20 '18 at 23:52

score 0 · Accepted Answer · answered Aug 19 '18 at 00:12

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.

Take a look at https://phpunit.readthedocs.io/en/7.3/writing-tests-for-phpunit.html#data-providers. — localheinz, Aug 19 '18 at 05:50
data providers is first to come to mind. you could create two test cases, one for happy path and one for not so happy at all or in part. — hakre, Aug 20 '18 at 21:38

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

2 Answers2