3

I've recently come across this code:

do {
    if ( ! checkSomething() )
        break;

    // some code

    if ( ! checkSomeOtherThing() )
        break;

    // some other code
} while(false);

// some final code

The programmer that wrote it, wrote a comment along the lines of "cleaner control flow".

In my opinion, the original code could look better if its refactored into something else. But is there any truth in this statement ? Is this construct any good ?

8
  • 2
    Into "something else". Exactly how are we supposed to judge this given you haven't defined "something else"?
    – Noon Silk
    Dec 6, 2010 at 0:08
  • 1
    I would say this is no better than putting a goto label before "some final code" and goto-ing it; at least with goto the label is explicit. This code is just kinda weird. Dec 6, 2010 at 0:11
  • @sje397 this may look like a loop, but it's not. See the loop condition.
    – robert
    Dec 6, 2010 at 0:12
  • Actually if you think about it, do ... while (false) means that you don't need the loop at all! Dec 6, 2010 at 0:13
  • @vic but break statements do need a loop.
    – robert
    Dec 6, 2010 at 0:15

4 Answers 4

6

I find this much easier to read, and it produces an identical result:

if ( checkSomething() )
{
    // some code
    if ( checkSomeOtherThing() )
    {
        // some other code
    }
}
// some final code

I think do ... while is normally hard to follow, but using it for something other than a loop is misleading at best.

2
  • 2
    And I find this far harder. I prefer to leave the control as early as possible. This is quite subjective.
    – Noon Silk
    Dec 6, 2010 at 0:12
  • While I think this code may seem harmless, if there are more breaks in the original piece it will make it far worse. But I do agree that the do ... while(false) construct is very misleading.
    – javs
    Dec 7, 2010 at 1:26
3

This is equivalent to a goto.

In such situations, it is better to use a goto than to use an ugly hack.

Changing it to use a goto makes it much more readable:

if (!checkSomething())
    goto Done;

// some code

if (!checkSomeOtherThing())
    goto Done;

// some other code
Done: //some final code
11
  • 1
    No, it's not equivalent to a goto. Goto can go to anywhere. This will only always go to the end of the loop. You may as well claim goto is always better then break or continue which is just madness.
    – Noon Silk
    Dec 6, 2010 at 0:27
  • 2
    A goto also goes to one place: A label. An explicitly named and descriptive label. Introducing a not-loop here just to use break as a thinly-disguised goto muddies the issue and is honestly the worst possible choice.
    – Anon.
    Dec 6, 2010 at 0:28
  • 2
    @robert: If it's large enough to need to search for the goto, then it's also so large that anyone finding the top of the loop isn't going to immediately realize that it's not actually a loop. Which is much worse, especially considering that any decent IDE can jump you directly to the label/point out exactly how far away it is.
    – Anon.
    Dec 6, 2010 at 0:36
  • 1
    @Anon thats exactly what happened to me at first: I had no clue of what was going on there (// some code was actually a bit large). I understood why he did it when I read the comment that was left there, and then I started looking for the breaks inside.
    – javs
    Dec 6, 2010 at 0:46
  • 1
    @Noon: While a proper refactoring is desirable, replacing the not-loop with goto s would still be an improvement, without taking the time that a proper refactoring would require. If your control flow pattern is a straight jump that doesn't fit into any other structured control flow pattern, then you should be using a goto, even if only to remind yourself that your code might be starting to smell a little.
    – Anon.
    Dec 6, 2010 at 0:49
2

If you don't mind loops containing several break statements, then the only problem here is that C (for obvious reasons) doesn't let you break out of a bare block, hence the "non-loop" which some unsuspecting future maintainer could mistake for a real loop.

The considerations, I think, are:

  • if there are only two break points, what's so bad about two if statements?
  • if there are more than two break points then the indentation with if statements could get unpleasant, and this saves that, but then again is the function doing too much? And even if not, would it be better just to use goto and avoid the weirdness of a loop that doesn't loop?

Since you tag this language-agnostic, I used to use a macroised assembly language, with a block ... endblock that you could break out of. This lead to reasonably nice code for checking necessary conditions, such as:

block
    breakif str1 == null
    breakif str2 == null
    get some combined property of str1 and str2
    breakif some other condition that stops us getting on with it
    get on with it
endblock

Actually, it wasn't breakif str1 == null, it was breakifeq.p str1, null, or something like that, but I forget exactly what.

0

I've seen the do-while form adopted as a standard to which coders conformed. The advantage is that it communicates, and implements, that the loop will always be executed at least once. This helps isolate, with consistency, the situations where something different occurs, i.e. where the code in the loop is not executed.

This standard was adopted because the Warnier-Orr technique was being applied.

1
  • I disagree with the communication part: there was a comment thrown there that actually told me what the guy tried to do with the do ... while(false);. You can easily communicate better by naming that piece of code by placing it in a function.
    – javs
    Dec 7, 2010 at 1:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.