Use of for loop conditional statement unrelated to iterating variable

Question

Is it considered decent form to write code like this:

int done = 0;
for (x = 0; !done; x++) {

    ... something involving 'x', that might end early ...

    if (!(x < max))
        done = 1;
}

as opposed to having to use a while loop:

int done;
x = 0;
while (!done) {

    ... something involving 'x', that might end early ...

    if (!(x < max))
        done = 1;
    x++;
}

or having to break out of a for loop:

for (x = 0; x < max; x++) {
    int done;

    ... something involving 'x', that might end early ...

    if (done)
        break;
}

---

I prefer the first one, mainly because I dislike breaking out of loops, I prefer for loops, and it still looks neat. The second one appears to me as what others would likely use.

The third one feels icky to me, but it seemed like something else that others might use as opposed to the first.

What are the merits of using one over another? Should they each be used at different times, is one generally correct, or is it just a matter of personal preference?

Answer 1

Consider the following:

int done = 0;
for (i = 0; !done && i < max; ++i) {
    // ... something involving 'i', that might end early ...
}

My personal preference here is to keep the "break condition" in one point. If you add a second if clause that breaks, you risk to skip over that logic when you or another programmer reads over it later. As a general rule, you should avoid using breaks in loops unless it is unavoidable. In a certain sense, it is the equivalent of a goto.

In this way the number of lines is minimized and, dare I say it, the code looks cleaner this way. Note that I used "i" in the place of "x" because if it is an index, as a rule, I tend to prefer i, though you can use whatever you prefer.

In general, if I have to cycle through a list or array of items, I will use a for loop, even if, as in your case, you may have conditional rules for exiting the loop. I will use a while loop if I am iterating through a list or array of items and index/position in list or array is not important. I will also use while if there is no predetermined end to the loop. That doesn't mean there isn't one, but rather that I don't know when the while loop will end before I begin.

I will generally avoid do ... while as more often than not, the very reason you cannot continue due to your break condition is what the logic contained within will depend on, and so you usually end up with something like:

condition = 0;
do {
    if(!condition) {
       // Perform some operation and possibly set condition to 1.
    }
} while (!condition);

I tend to at that point break the operation into a function of its own:

while(performOperation()) ;

...

int performOperation() {
    // Set result to value indicating success or failure
    return result;  
}

Hope that helps!

Answer 2

Different blocks have different meanings: by using the ones which are commonly used for a specific purpose, you make your code easier to read and maintain. By using the ones which are not commonly used for the purpose, you force readers of your code to stop and think why you've used this construct instead of the commonly used one.

While semantically correct, for (x = 0; !done; x++) is not something usual. while or do...while are used in situations where you need to break from a loop when a condition is met:

x = 0;
do {
    ... something involving 'x', that might end early.
    x++;
}
while (x <= max)

(assuming that max can change within the loop; otherwise, you can simply do for (x = 0; x <= max; x++))

Being concise is important too. Less code means less risk of errors. For example, your while-based code is unnecessarily complicated. For example, (!(x < max)) takes time to understand and may be source of an error. The if block within the while adds complexity too. You also add a variable which transcends the while block and which can be simply removed.

---

I prefer the first one, mainly because I dislike breaking out of loops

That is your personal opinion which is valuable when you're working on a personal project which will never be shared with anybody. As soon as you think that the code may be open sourced (and actually read and used by others) or your work in a team, what you prefer may go against:

• Your team habits. If in a team of 6 persons, 5 are using a construct and you are using a different construct, for the sake of consistency, do use the one your teammates use, including if yours looks clearly superior (unless of course you can convince your teammates at the beginning of the project that your's is better).

• Community usages. Different communities use different styles, which leads to discrepancies between languages. For example:

  if (something) {
  }
  

  in JavaScript should be written:

  if (something)
  {
  }
  

  in C#. Sticking to community usages makes it easier for the people outside your project to read your code (for example if you post a piece of code on Stack Overflow) or for you to read code from other projects.

• Official style and style checkers. You may disagree with the choice the authors of the style guide made, but the fact is, consistency is more important.

  For example, I find a few rules of PSR-2 quite weird. For example:

  function hello()
  {                      // Newline here.
      if (something) {   // No newline here.
      }
  }
  

  Still, I'll rather use this style than create my own and find myself inconsistent with thousands of PHP programmers.

Answer 3

The first form

for ( x = 0; !done; x++ )

is a little weird and non-intuitive IMO. There's no obvious correlation between x and done in this context (you have to review the body of the loop to see that done is set when x is no longer less than max)

In that case, I would much rather see something like

for ( x = 0; x < max; x++ )  

However, you're concerned about the loop terminating before x is equal to max due to some other condition. If you want to terminate the loop as soon as the other condition is true, you could do something like the following:

for ( x = 0; x < max && !other_exit_condition; x++ )
{
  // do the thing with x that may set other_exit_condition
  if ( other_exit_condition )
    continue; 
  else
  {
    // rest of loop body
  }
}

The continue statement goes back to the beginning of the loop, which then exits because other_exit_condition is now true. But this is really no better than using a break, honestly.