views:

204

answers:

7

My group is having some discussion and strong feelings about for loop construction.

I have favored loops like:

   size_t x;
   for (x = 0; x < LIMIT; ++x) {
      if (something) {
          break;
      }
      ...
   }

   // If we found what we're looking for, process it.
   if (x < LIMIT) {
       ...
   }

But others seem to prefer a Boolean flag like:

   size_t x;
   bool found = false;
   for (x = 0; x < LIMIT && !found; ++x) {
      if (something) {
          found = true;
      }
      else {
          ...
      }
   }

   // If we found what we're looking for, process it.
   if (found) {
       ...
   }

(And, where the language allows, using "for (int x = 0; ...".)

The first style has one less variable to keep track of and a simpler loop header. Albeit at the cost of "overloading" the loop control variable and (some would complain), the use of break.

The second style has clearly defined roles for the variables but a more complex loop condition and loop body (either an else, or a continue after found is set, or a "if (!found)" in the balance of the loop).

I think that the first style wins on code complexity. I'm looking for opinions from a broader audience. Pointers to actual research on which is easier to read and maintain would be even better. "It doesn't matter, take it out of your standard" is a fine answer, too.

OTOH, this may be the wrong question. I'm beginning to think that the right rule is "if you have to break out of a for, it's really a while."

bool found = false;
x = 0;
while (!found  && x < LIMIT) {
    if (something) {
        found = true;
        ...handle the thing...
    }
    else {
        ...    
    }
    ++x;
}

Does what the first two examples do but in fewer lines. It does divide the initialization, test, and increment of x across three lines, though.

A: 

This looks like a place for a while loop. For loops are Syntactic Sugar on top of a While loop anyway. The general rule is that if you have to break out of a For loop, then use a While loop instead.

WolfmanDragon
Whose general rule would that be? Whether you break out or not is irrelevant to the loop construct, unless you rely on the looping variable afterward, which is a Bad Thing.
T.J. Crowder
Computer Science 101.
WolfmanDragon
20+ years, never heard that one, and don't buy it.
T.J. Crowder
I also have never heard of a reason why putting a break in a for loop is a bad thing. In fact, a for loop with a break is typically what I would recommend when trying to find an element in an array.
Jacob Adams
Actually, "if you have to break out of a for loop, then use a while loop instead" is starting to sound good to me. Arguably, searches should be whiles.
Chris Nelson
@Jacob: The argument against breaks is that they aren't structured. Structured Programming requires that every block has exactly one entry point and exactly one exit point. break statement do not follow this rule. They're not as bad as goto... It's probably about the same level of "sin" as multiple return statements from a function, which a lot of people also don't care about...
Brian Postow
I would so love to see my old instructors faces if any of them saw these posts promoting breaks inside a loop. :) Sure we can use a hammer to drive in a screw, but maybe we should use a screwdriver instead.
WolfmanDragon
+1  A: 

While I disagree that an extra else really makes the 2nd more complicated, I think it's primarily a matter of aesthetics and keeping to your standard.

Personally, I have a probably irrational dislike of breaks and continues, so I'm MUCH more likely to use the found variable.

Also, note that you CAN add the found variable to the 1st implementation and do

if(something)
{
   found = true;
   break;
}

if you want to avoid the variable overloading problem at the expense of the extra variable, but still want the simple loop terminator...

Brian Postow
+1  A: 

The former example duplicates the x < LIMIT condition, whereas the latter doesn't.

With the former, if you want to change that condition, you have to remember to do it in two places.

RichieHindle
Ah, but the second example have the "same" condition twice but expressed differently (!found in the loop header, found in the post condition). That, to me, is worse because it's harder to notice with a visual or programmatic scan.
Chris Nelson
A: 

This sort of argument has been fought out here before (probably many times) such as in this question.

There are those that will argue that purity of code is all-important and they'll complain bitterly that your first option doesn't have identical post-conditions for all cases.

What I would answer is "Twaddle!". I'm a pragmatist, not a purist. I'm as against too much spaghetti code as much as the next engineer but some of the hideous terminating conditions I've seen in for loops are far worse than using a couple of breaks within your loop.

I will always go for readability of code over "purity" simply because I have to maintain it.

paxdiablo
A: 

I don't have any references to hand (-1! -1!), but I seem to recall that having multiple exit points (from a function, from a loop) has been shown to cause issues with maintainability (I used to know someone who wrote code for the UK military and it was Verboten to do so). But more importantly, as RichieHindle points out, having a duplicate condition is a Bad Thing, it cries out for introducing bugs by changing one and not the other.

If you weren't using the condition later, I wouldn't be bothered either way. Since you are, the second is the way to go.

T.J. Crowder
What is "(-1! -1!)"?
paxdiablo
A (clearly unsuccessful) joke, e.g., vote this down, he failed to provide references. :-)
T.J. Crowder
I think I would prefer a simple [citation needed] B-)
Brian Postow
Aah, that makes more sense. I was trying to figure out what (-1) factorial minus 1 factorial was but of course I got stuck on the factorial of a negative number :-)
paxdiablo
+1  A: 

I would prefer a different one altogether:

       for (int x = 0; x < LIMIT; ++x) {
          if (something) {
              // If we found what we're looking for, process it.
              ...
              break;
          }
          ...
       }

It seems you have not any trouble you mention about one or the other... ;-)

  • no duplication of condition, or readability problem
  • no additional variable
KLE
That's interesting. Of course, sometimes the "..." is "return x" and returning from the middle of a loop feels grody.
Chris Nelson
@Chris You saw I didn't change the '...'. This was in a condition already, so it wasn't on the last line of the method. ;-) Anyway, I think the number of returns is another debate...
KLE
+1. Even if the `...` was `return x;` I'd have no problem with this (other than removing the `break;` of course). If `...` is complicated just re-factor it as another method.
Grant Wagner
+1  A: 

I'd actually dare to suggest consideration of GOTO to break out of loops in such cases:

   for (size_t x = 0; x < LIMIT && !found; ++x) {
      if (something) 
          goto found;

      else {
          ...
      }
    }

    // not found
           ...

      return;

    found:

       ...
      return;

I consider this form to be both succint and readable. It may do some good in many simple cases (say, when there is no common processing in this function, in both found/unfound cases).

And about the general frowning goto receives, I find it to be a common misinterpretation of Dijkstra's original claims: his arguments favoured structured loop clauses, as for or while, over a primitive loop-via-goto, that still had a lot of presence circa 1968. Even the almighty Knuth eventualy says -

The new morality that I propose may perhaps be stated thus: "Certain go to statements which arise in connection with well-understood transformations are acceptable, provided that the program documentation explains what the transformation was."

Others here occasionaly think the same.

Ofek Shilon
Mein Gott! People have been abused here for using break and continue and you do sully forth with a goto - you are brave :-)
paxdiablo
My head is humbly bent-down for the downvotes that are sure to follow. :)
Ofek Shilon
In fact, just found accidently another supporter here: http://stackoverflow.com/questions/1258201/will-using-goto-cause-memory-leaks/1258270#1258270
Ofek Shilon