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

Discussion in 'C Programming' started by benmarco7@mail.com, Apr 2, 2008.

  1. Guest

    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
    , Apr 2, 2008
    #1
    1. Advertising

  2. wrote:
    > CODE COMPLETE, page 356
    > if(StatusOK)
    > {
    > if(DataAvail)
    > {
    > ImportantVar = x;
    > goto MID_LOOP;
    > }
    > }
    > else
    > {
    > ImportantVar = GetVal( );
    > MID_LOOP;
    >
    > /* lots of code */
    > . . .
    > }
    >
    > * * * * * * end fragment * * * * *
    >

    [much snippage below]

    > The idea here is to rewrite the code without the goto LABLE.
    >
    > 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.
    >
    > Either this is a defect, or we have some other intent (IntentB):
    > Do the Work under all conditions, except *this one*.

    ["This one" being (StatusOK && !DataAvail)]

    Good analysis, but I disagree with your conclusions. Changing behaviour
    is a dangerous game in program maintenance.

    You identify two possible intents (which you call IntentA and IntentB)
    in what the code /should/ do. You correctly note that IntentB is what
    the code actually does, though IntentA is possibly what the code should do.

    Now there are four possibilities:

    1. You change behaviour to IntentA and IntentA is correct:
    Congratulations, you have removed the "goto" and corrected a bug at
    the same time!

    2. You change the behaviour to IntentA but IntentB is correct:
    Oh dear! You have removed the goto, but introduced a bug. The
    program is neater but is now incorrect. (see footnote [1]) This is a
    regression, which is a Bad Thing.

    3. You keep the behaviour as IntentB but IntentA is correct:
    Well done! You have improved the code by removing a goto, and you
    have neither introduced nor fixed any bugs. Possibly more work needs to
    be done.

    4. You keep the behaviour as IntentB and IntentB is correct:
    Congratulations, you have removed the "goto"! The code is now neat
    and correct.

    Now, these possibilities are not equiprobable, but it can be seen that
    by maintaining the current behaviour (IntentB) you cannot introduce any
    new bugs into the program. Possibilities 1. and 4. are both ideal - the
    program ends up neater and bug-free. The interesting ones are
    possibilities 2. and 3.: the program is neater but incorrect. However,
    in possibility 2., you started with a perfect program and you broke it
    by introducing a bug; in possibility 3., you started with a buggy
    program and made the code neater. 3. is an improvement, while 2. is a
    regression.

    By keeping the behaviour the same, only 3. or 4. are possible, and both
    are Good Things. By changing the behaviour, 1. or 2. are possible; 1. is
    Good, but 2. is Bad.

    What this boils down to is you need to be absolutely sure that IntentA
    is the correct behaviour before you attempt to modify the code's
    behaviour. Otherwise you run the risk of introducing bugs into a program
    which was working fine before. How can you be sure what the correct
    behaviour is? You look at the formal specification, you examine the code
    in context, you look at what the caller expects this code to do. Since
    this is an exercise, I doubt that a formal spec exists, or even a
    caller. So I'd play it safe and keep the behaviour the same.

    DISCLAIMER: I haven't read "Code Complete"; I don't know the wider
    context of the exercise. This is based entirely on what you wrote in
    your article.

    Philip

    [1] "Complex problems often have simple, easy to understand, wrong answers."
    Philip Potter, Apr 3, 2008
    #2
    1. Advertising

  3. Re: A conundrum in C: found this fragment in Steve McConnell's CODE COMPLETE

    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.
    Ben Bacarisse, Apr 3, 2008
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Simple Simon

    P.O. Box Validation -- Hans?...Steve(Guru)?

    Simple Simon, Nov 9, 2003, in forum: ASP .Net
    Replies:
    3
    Views:
    3,308
    Vishnu Sistla
    Oct 9, 2011
  2. ¿À¼º¿µ

    thanks you , Steve...

    ¿À¼º¿µ, Feb 9, 2006, in forum: ASP .Net
    Replies:
    0
    Views:
    377
    ¿À¼º¿µ
    Feb 9, 2006
  3. Richard

    Steve Pugh

    Richard, Nov 12, 2003, in forum: HTML
    Replies:
    9
    Views:
    617
    informant
    Nov 14, 2003
  4. Dave Benjamin

    Guido interviewed by Steve Holden

    Dave Benjamin, Aug 17, 2003, in forum: Python
    Replies:
    0
    Views:
    559
    Dave Benjamin
    Aug 17, 2003
  5. Dave Harrison

    Re: Guido interviewed by Steve Holden

    Dave Harrison, Aug 17, 2003, in forum: Python
    Replies:
    0
    Views:
    567
    Dave Harrison
    Aug 17, 2003
Loading...

Share This Page