Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is the cylcomatic complexity of this function 12?

I have a (C#) function that checks four sets of conditions and returns a bool. If any of them are true, it returns true. I'm sure I could simplify the logic but I want it to be fairly readable.

The CodeMaid extension in Visual Studios and tells me the cylomatic complexity of the function is 12. I looked it up and the cylomatic complexity is

the number of independent paths through the source code

I don't understand why it's 12. I can think of it two ways, either the cyclomatic complexity should be 2, because it always goes through the same path but could return either a true or a false. Or could understand if it was 16, because the four booleans or'd together at the end could each be true or false, 2*2*2*2 = 16.

Can someone tell me why its 12? Maybe even show a diagram so I can visualize the different paths?

public bool FitsCheckBoxCriteria(TaskClass tasks)
{
    // note: bool == true/false comparisons mean you don't have to cast 'bool?' as bool


    // if neither checkboxes are checked, show everything
    bool showEverything = NoShutDownRequiredCheckBox.IsChecked == false &&
                          ActiveRequiredCheckBox.IsChecked == false; 

    // if both are checked, only show active non-shutdown tasks
    bool showActiveNonShutdown = ActiveRequiredCheckBox.IsChecked == true &&
                                 tasks.Active == "YES" &&
                                 NoShutDownRequiredCheckBox.IsChecked == true &&
                                 tasks.ShutdownRequired == "NO";

    // if active is checked but shudown isn't, display all active
    bool showActive = ActiveRequiredCheckBox.IsChecked == true &&
                      tasks.Active == "YES" &&
                      NoShutDownRequiredCheckBox.IsChecked == false;

    // if non-shutdown is checked but active isn't, display all non-shutdown tasks
    bool showNonShutdown = NoShutDownRequiredCheckBox.IsChecked == true &&
                           tasks.ShutdownRequired == "NO" &&
                           ActiveRequiredCheckBox.IsChecked == false;

    return showEverything || showActiveNonShutdown || showActive || showNonShutdown;
}

Thanks in advance.

Edit:

I changed it to this. assigning local variables for the checkbox conditions didn't have any effect, but creating booleans out of the "YES"/"NO" cranked up the complexity to 14, which I think I understand.

public bool FitsCheckBoxCriteria(LubeTask tasks)
{
    bool noShutdownReqChecked = (bool)NoShutDownRequiredCheckBox.IsChecked;
    bool activeChecked = (bool)ActiveRequiredCheckBox.IsChecked;

    bool active = tasks.Active == "YES" ? true : false;
    bool shutdownReq = tasks.ShutdownRequired == "YES" ? true : false;

    // if neither checkboxes are checked, show everything
    bool showEverything = !noShutdownReqChecked && !activeChecked;

    // if both are checked, only show activeChecked non-shutdown tasks
    bool showActiveNonShutdown = activeChecked && noShutdownReqChecked && active && !shutdownReq;

    // if activeChecked is checked but shudown isn't, display all activeChecked
    bool showActive = activeChecked && !noShutdownReqChecked && active;

    // if non-shutdown is chceked but activeChecked isn't, display all non-shutdown tasks
    bool showNonShutdown = noShutdownReqChecked && !activeChecked && !shutdownReq;

    return showEverything || showActiveNonShutdown || showActive || showNonShutdown;
}
like image 503
Charles Clayton Avatar asked Apr 23 '15 20:04

Charles Clayton


People also ask

Is there a cyclomatic complexity of 10?

If a method has a cyclomatic complexity of 10, it means there are 10 independent paths through the method. This implies is that at least 10 test cases are needed to test all the different paths through the code. The lesser the number, the easier it is to test.

How do you find the cyclomatic complexity of a function?

Apply formulas in order to compute Cyclomatic complexity. 3) Cyclomatic complexity V(G) = P +1 V (G) = 2 + 1 = 3 Where P is predicate nodes (node 1 and node 2) are predicate nodes because from these nodes only the decision of which path is to be followed is taken. Thus Cyclomatic complexity is 3 for given code.

Why do we calculate cyclomatic complexity?

It can be used as a quality metric, gives relative complexity of various designs. It is able to compute faster than the Halstead's metrics. It is used to measure the minimum effort and best areas of concentration for testing. It is able to guide the testing process.

What cyclomatic complexity is too high?

Consequences: A high cyclomatic complexity for a particular function means that the function will be difficult to understand, and more difficult to test. That in turn means functions with a cyclomatic complexity above 10 or 15 can be reasonably expected to have more undetected defects than simpler functions.


2 Answers

The key is in "independent paths".

I'm going to rewrite your code to shorten it so we can discuss it.

public bool FitsCheckBoxCriteria(TaskClass tasks)
{
    bool E1 = A1 && A2; 

    bool E2 = B1 && B2 && B3 && B4;

    bool E3 = C1 && C2 && C3;

    bool E4 = D1 && D2 && D3;

    return E1 || E2 || E3 || E4;
}

The Cyclomatic complexity is the number of independent paths. This is not the total possible number of return values (2).

The && operator and the || operator are short circuit operations; if A1 is false, A2 is not evaluated. Similarly, if E1 is true, E2 is not evaluated.

If you replace all the &&s with & and all the ||s with | in the code above, the cyclomatic complexity is 1, because there is only one path through the code. (This wouldn't make it better code though).

As it is, there are 72 possible paths...

  1. A1, B1, C1, D1, E1 are evaluated; the others are not.
  2. A1, A2, B1, C1, D1, E1 are evaluated; the others are not.
  3. A1, B1, B2, C1, D1, E1 are evaluated; the others are not.
  4. A1, A2, B1, B2, C1, D1, E1 are evaluated; the others are not. etc...

But path 4 doesn't contain any new code that isn't already on the previous paths. And this is the definition of "independent paths" - each path must include new code.

So in this example, you can count the code by hand as follows:

1 + the number of short circuit operators in this code (11) = 12.

Wikipedia has an excellent in depth explanation.

like image 164
perfectionist Avatar answered Oct 03 '22 07:10

perfectionist


This is just a guess, but I think the assignments are +2 each (if =true/else =false) and then +1 for each possible exit condition in the return statement. So it might unwind into something like:

bool showEverything = false;
if (...) { showEverything = true; } +1
else { showEverything = false; } +1

bool showActiveNonShutdown = ... +2 if/else
bool showActive = ... +2 if/else
bool showNonShutdown = ... +2 if/else

if (showEverything) {...} +1
else if (showActiveNonShutdown) {...} +1
else if (showActive) {...} +1
else if (showNonShutdown) {...} +1
else {false}
like image 40
doogle Avatar answered Oct 03 '22 06:10

doogle