Why is the cylcomatic complexity of this function 12?

Question

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

Just curious, what happens to the complexity if you delete all the `== true` statements and change `== false` to `!`? They're redundant — moarboilerplate, Apr 23 '15 at 21:16
@moarboilerplate I know they're redundant. It's just that those are type `bool?` not `bool`, so instead of casting all of them, I just did a comparison for my readability's sake. https://msdn.microsoft.com/en-us/library/bb384091.aspx — Charles Clayton, Apr 23 '15 at 21:19
Yeah, but what does it do to the complexity? Also, if you're interested in optimizing this I would use a flags enum and bitwise operations to set a permission for each bit. — moarboilerplate, Apr 23 '15 at 21:23
(although that does take away the nullable aspect of your boolean) — moarboilerplate, Apr 23 '15 at 21:23
Hmm, I got rid of the comparisons and did casts and it didn't change the complexity. That would have been neat though. — Charles Clayton, Apr 23 '15 at 21:27
Yeah, since you are comparing them to constant values the compiler is usually smart enough to figure that out :/ — moarboilerplate, Apr 23 '15 at 21:28

score 3 · Answer 1 · answered Apr 23 '15 at 20:48

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}

That's a very interesting way of looking at it! – Charles Clayton Apr 23 '15 at 20:56 — Charles Clayton, Apr 23 '15 at 20:56

score 3 · Accepted Answer · answered Apr 27 '15 at 08:50

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...

A1, B1, C1, D1, E1 are evaluated; the others are not.
A1, A2, B1, C1, D1, E1 are evaluated; the others are not.
A1, B1, B2, C1, D1, E1 are evaluated; the others are not.
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.

score 2 · Answer 3 · answered Apr 26 '15 at 07:35

C# uses short-circuit evaluation which means that if there is an x && y expression y is evaluated only if it is necessary, more precisely, if x is true. This means that result = x && y; has two independent execution paths: (1) if x is false then only x is evaluated and result gets false value (without evaluating y) but (2) if x is true y is evaluated as well and result gets the evaluation result of y. This means that every && and || operator increases the cyclomatic complexity and in the first example there are 8 && and 3 || operators so the cyclomatic complexity of the method is 12.

Why is the cylcomatic complexity of this function 12?

3 Answers3