A conundrum in C: found this fragment in Steve McConnell's CODECOMPLETE

[benmarco7]

CODE COMPLETE, page 356

if(StatusOK)
{
if(DataAvail)
{
ImportantVar = x;
goto MID_LOOP;
}
}
else
{
ImportantVar = GetVal( );
MID_LOOP;

/* lots of code */
. . .
}

* * * * * * end fragment * * * * *

The idea here is to rewrite the code without the goto LABLE.

First we identify the Work.
Initialize or re-initialize the variable ImportantVar,
conditionally,
in the if or the else clause.
And,
execute /* lots of code */ unconditionally, no matter what the
conditions are.

Reading the code is not the problem, or even writing code.
The real problem is in divining the intent of the coder.

Let us say the intent (IntentA) is to do the Work under any
conditions.
Then this fragment has a problem.
In the event that SatusOK is true, but DataAvail is false, then the
outer
if will ( since its own test is ture ) execute its code block, the
inner if,
but will fall thru, doing no Work.
The else is the companion clause of the outer if, and since the if
tried
to execute, the else will be skipped.
No Work will be done at all.

Either this is a defect, or we have some other intent (IntentB):
Do the Work under all conditions, except *this one*.

Here is my rewirte for IntentA, a bit simpler than what McConnell
offered.
**********************************
If( StatusOK && DataAvail )
{
ImportantVar = x;
}
else
{
ImportantVar = GetVal( );
}

/* lots of code */ // What is the sense of putting
unconditionally

[Ben Bacarisse]

be*******@mail.com writes:

CODE COMPLETE, page 356

if(StatusOK)
{
if(DataAvail)
{
ImportantVar = x;
goto MID_LOOP;
}
}
else
{
ImportantVar = GetVal( );
MID_LOOP;

Presumably this is intended to be MID_LOOP:

/* lots of code */
. . .
}

The sort of "obvious" re-write is just to separate the setting of teh
variable from the execution of the labeled code block. To my mind it
makes thing much clearer. Personally, I'd invert the first test
because it makes the setting simpler:

if (!StatusOK)
ImportantVar = GetVal();
else if (DataAvail)
ImportantVar = x;

if (!StatusOK || DataAvail) {
/* lots of code */
}

Of course, if the tests are complex so that re-evaluating them is not
acceptable then I'd be thinking about adding a new function.

--
Ben.