How to convert a function which failed MCDC to one that does?

According to Wiki (https://en.wikipedia.org/wiki/Modified_condition/decision_coverage) one of the main rules about MCDC is that each condition (or boolean variable) should be controllable independently of the others, to ensure that decision paths through code are independently testable. In the below I have a failing example:

int FooBar(int n, bool p, bool q)
{
    int out = 34;

    if (n == 123)
    {
       if (p || q)  out = 4;
       if (p && q)  out = 9;
    }

    return out;
}

The problem here is that p || q and p && q are not independent conditions, e.g. if the first is false then so is the second, and if the second is true then the first is true too.

How could I rewrite this function to give identical behaviour but be MCDC-compliant? My best guess is the following:

int FooBar(int n, bool p, bool q)
{
    int out;

    if (n == 123)
    {
       if (p)
       {
           if (q) out = 9;
           else   out = 4;
       }
       else
       {
           if (q) out = 4;
           else   out = 34;
       }
    }
    else
    {
        out = 34;
    }

    return out;
}

but would this be valid? The only obvious conditions here are three conditions p, q, and n==123, which are indeed independent, right? Have I understood MCDC right? I'm also nervous about the fact that we have two branches that both lead to out = 4;, i.e. two branches that lead to the identical outcomes, is this valid?

Solution

You seem to have some basic uncertainty about MC/DC, and perhaps some misconceptions. NASA has a tutorial on MC/DC that might help.

The primary thing you need to understand, though, is that MC/DC is a test-coverage metric, not a set of code style rules. Although a requirement for 100% MC/DC coverage has implications for code structure, it is primarily about what test cases need to be exercised.

MC/DC terminology

one of the main rules about MCDC is that each condition (or boolean variable) should be controllable independently of the others

Yes, and here it is important to understand that in MC/DC terminology, "condition" means a boolean expression that does not contain any boolean operators. That can be either a boolean variable / parameter (p, q) or a relational or (in)equality expression (n == 123). It is furthermore important to understand that here, when cast in terms of source code, expression means a specific sequence of tokens. A specific appearance of the name of parameter p, for example. Thus, every condition is unique.

On the other hand, "decision" means a boolean expression composed of conditions and zero or more boolean operators (so conditions are also decisions).

MC/DC rules

The MC/DC rule that distinguishes it from C/DC is that on a per-decision basis, each condition appearing in the decision is controllable independently of the others. The NASA tutorial explains that this is often verified by applying the "unique-cause" approach, wherein the code under test is exercised for each condition by varying that condition while holding the others constant. It observes, however, that that does not work for decisions that contain duplicate or strongly-coupled conditions, such as (A && B) || (A && C). Each of of the As in that decision is a separate condition, and one cannot be varied while holding the other constant. Your function does not have any such issue.

You write:

The problem here is that p || q and p && q are not independent conditions,

Yes and no. Those indeed are not independent conditions, but that's because they are not conditions at all (though they are decisions).

So what does full MC/DC coverage require for a test set for this function?

every condition in a decision in the program has taken all possible outcomes at least once. This requires a set of test cases in which

n == 123 evaluates to true at least once and to false at least once
p || q evaluates to true at least once and to false at least once (which requires this decision to be evaluated at all)
p && q evaluates to true at least once and to false at least once (which requires this decision to be evaluated at all)

The "evaluates to" in the above means that control reaches the decision in question and its evaluation produces the specified result. It cannot be determined outside the context of the structure of the function, based on the parameter values alone.

each condition has been shown to affect that decision outcome independently

For n == 123, the decision is a condition, so this requirement is satisfied as long as outcome is tested by at least one test case
For p || q, that decision must be evaluated for (i) different values of p and the same value of q, AND (ii) different values of q and the same value of p.
For p && q, that decision must be evaluated for (i) different values of p and the same value of q, AND (ii) different values of q and the same value of p.

Note that with the code as originally structured, any set of test cases that fulfills this condition for p || q also fulfills it for p && q.

Thus, this set of test cases suffices:

#	n	p	q
1	0	false	false
2	123	false	false
3	123	true	false
4	123	true	true

Specifically

n == 123 evaluates to false in case 1 and to true in the other cases, so this decision takes every possible outcome at least once.
p || q evaluates to false in case 2 (but not in case 1, where it is not evaluated at all!), and to true in cases 3 and 4, so this decision takes every possible outcome at least once
p && q evaluates to false in cases 2 and 3 (but not in case 1), and to true in case 4, so this decision takes every possible outcome at least once
for n == 123 considered as a decision, the n == 123 condition is tested independently by the combination of cases 1 and 2. (Or 1 and 3 or 1 and 4 -- the values of p and q are irrelevant to this particular question because the decision does not involve them.)
for both p || q and p && q, the respective p condition is tested independently by the combination of cases 2 and 3
for both p || q and p && q, the respective q condition is tested independently by the combination of cases 3 and 4

That set also happens to exercise all the possible control-flow paths through the function, which, though a good characteristic, is not a criterion of MC/DC, nor a characteristic that necessarily follows from a test set that exhibits full MC/DC coverage.

Were it me, however, I'd be inclined to add one more test to reflect the symmetry of the || and && decisions:

#	n	p	q
5	123	false	true

Adding that to the others goes a bit beyond MC/DC for the sake of a more thorough test suite. After all, MC/DC analysis is just a tool for helping ensure good test coverage. That's only one criterion for a test suite.

Other considerations

I'm also nervous about the fact that we have two branches that both lead to out = 4;, i.e. two branches that lead to the identical outcomes, is this valid?

MC/DC "outcome" is not about the program behavior that proceeds from any decision or decisions. It is the result itself of evaluating a condition or decision. If multiple distinct combinations of conditions produce the same observable result then restructuring the code is not going to fix that.

As a separate matter, if you're concerned about (or your test tool is unhappy with) the fact that the outcome of the p || q decision is masked when p && q evaluates to true, then the most likely workaround would be simply to restructure the tests as an if / else if (in reverse order):

       if (p && q)  out = 9;
       else if (p || q)  out = 4;

Then you definitely want case (5) in your test set along with the others, as cases 3, 4, and 5 would provide MC/DC for the p && q decision, whereas 2, 3, and 5 would provide it for the p || q decision (that decision not even being evaluated in case 4).

Caveat

All the above is about what MC/DC actually measures. It is possible that your particular analysis tool takes a narrower view, or that it puts additional rules in the MC/DC category that aren't actually required by MC/DC at all. Or even that it's MC/DC analysis is buggy.