Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Consider the following code:

public void doSomething(int input)
{
   while(true)
   {
      TransformInSomeWay(input);

      if(ProcessingComplete(input))
         break;

      DoSomethingElseTo(input);
   }
}

Assume that this process involves a finite but input-dependent number of steps; the loop is designed to terminate on its own as a result of the algorithm, and is not designed to run indefinitely (until cancelled by an outside event). Because the test to see if the loop should end is in the middle of a logical set of steps, the while loop itself currently doesn't check anything meaningful; the check is instead performed at the "proper" place within the conceptual algorithm.

I was told that this is bad code, because it is more bug-prone due to the ending condition not being checked by the loop structure. It's more difficult to figure out how you'd exit the loop, and could invite bugs as the breaking condition might be bypassed or omitted accidentally given future changes.

Now, the code could be structured as follows:

public void doSomething(int input)
{
   TransformInSomeWay(input);

   while(!ProcessingComplete(input))
   {
      DoSomethingElseTo(input);
      TransformInSomeWay(input);
   }
}

However, this duplicates a call to a method in code, violating DRY; if TransformInSomeWay were later replaced with some other method, both calls would have to be found and changed (and the fact that there are two may be less obvious in a more complex piece of code).

You could also write it like:

public void doSomething(int input)
{
   var complete = false;
   while(!complete)
   {
      TransformInSomeWay(input);

      complete = ProcessingComplete(input);

      if(!complete) 
      {
         DoSomethingElseTo(input);          
      }
   }
}

... but you now have a variable whose only purpose is to shift the condition-checking to the loop structure, and also has to be checked multiple times to provide the same behavior as the original logic.

For my part, I say that given the algorithm this code implements in the real world, the original code is the most readable. If you were going through it yourself, this is the way you'd think about it, and so it would be intuitive to people familiar with the algorithm.

So, which is "better"? is it better to give the responsibility of condition checking to the while loop by structuring the logic around the loop? Or is it better to structure the logic in a "natural" way as indicated by requirements or a conceptual description of the algorithm, even though that may mean bypassing the loop's built-in capabilities?

share|improve this question
    
Would this question be better on: codereview.stackexchange.com/questions ? –  FrustratedWithFormsDesigner Mar 29 '12 at 18:46
10  
Possibly, but despite the code samples the question being asked is IMO more conceptual, which is more in line with this area of SE. What is a pattern or anti-pattern is discussed here often, and that's the real question; is structuring a loop to run infinitely and then breaking out of it a bad thing? –  KeithS Mar 29 '12 at 18:48
    
Good point, I guess if you posted actual code then CR might be more appropriate. –  FrustratedWithFormsDesigner Mar 29 '12 at 18:49
3  
The do ... until construct is also useful for these kinds of loops since they don't have to be "primed." –  Blrfl Mar 29 '12 at 18:57
2  
The loop-and-a-half case still lacks any really good answer. –  Loren Pechtel Mar 29 '12 at 23:25
show 2 more comments

7 Answers

up vote 5 down vote accepted

Stick with your first solution. Loops can become very involved and one or more clean breaks can be a lot easier on you, anyone else looking at your code, the optimizer, and the program's performance than elaborate if-statements and added variables. My usual approach is to set up a loop with something like the "while (true)" and get the best coding I can. Often I can and do then replace the break(s) with a standard loop control; most loops are pretty simple, after all. The ending condition should be checked by the loop structure for the reasons you mention, but not at the cost of adding whole new variables, adding if-statements, and repeating code, all of which are even more bug prone.

share|improve this answer
    
"if-statements, and repeating code, all of which are even more bug prone." -- yep when trying people I call if's "future bugs" –  Pat Mar 30 '12 at 19:19
add comment

Your alternatives are not as bad as you may think. Good code should:

1) Clearly indicate with good variable names / comments under what conditions the loop should be terminated. (The breaking condition should not be hidden)

2) Clearly indicate what is the order of doing operations. This is where putting the optional 3rd operation ( DoSomethingElseTo(input) ) inside the if is a good idea.

That said, it is easy to let perfectionist, brass-polishing instincts consume a lot of time. I would just tweak the if as mentioned above; comment the code and move on.

share|improve this answer
    
I think perfectionist, brass-polishing instincts save time in the long run. Hopefully, with practice, you develop such habits so your code is perfect and its brass polished without consuming a lot of time. But unlike physical construction, software can last forever and be used in an infinite number of places. The savings from code that's even 0.00000001% faster, more reliable, usable, and maintainable can be large. (Of course, most of my much-polished code is in languages long obsolete or making money for someone else these days, but it did help me develop good habits.) –  RalphChapin Mar 30 '12 at 17:53
    
Well I can't call you wrong :-) This is an opinion question and if good skills came out of the brass-polishing: great. –  Pat Mar 30 '12 at 19:20
    
I think its a possibility to consider, anyway. I certainly wouldn't want to offer any guarantees. It does make me feel better to think that my brass-polishing improved my skills, so I may be prejudiced on this point. –  RalphChapin Apr 1 '12 at 0:37
add comment

It's only a meaningful problem if the loop body is non-trivial. If the body is so small, then analyzing it like this is trivial, and not a problem at all.

share|improve this answer
add comment

I've trouble to find the other solutions better than the one with break. Well, I prefer the Ada way which allows to do

loop
  TransformInSomeWay(input);
exit when ProcessingComplete(input);
  DoSomethingElseTo(input);
end loop;

but the break solution is the cleanest rendering.

My POV is that when there is a missing control structure, the cleanest solution is to emulate it with other control structures, not using data flow. (And similarly, when I have a data flow problem to prefer pure data flow solutions to the one involving control structures *).

It's more difficult to figure out how you'd exit the loop,

Don't be mistaken, the discussion wouldn't take place if the difficulty wasn't mostly the same: the condition is the same and present at the same place.

Which of

while (true) {
   XXXX
if (YYY) break;
   ZZZZ;
}

and

while (TTTT) {
    XXXX
    if (YYYY) {
        ZZZZ
    }
}

render better the intended structure? I'm voting for the first, you don't have to check that the conditions matches. (But see my indentation).

share|improve this answer
1  
Oh, look, a language that does directly support mid-exit loops! :) –  mjfgates Mar 30 '12 at 16:47
add comment

while(true) and break is a common idiom. It's the canonical way to implement mid-exit loops in C-like languages. Adding a variable to store the exit condition and an if() around the second half of the loop is a reasonable alternative. Some shops may Officially Standardize on one or the other, but neither one is superior; they are equivalent.

A good programmer working in these languages should be able to know what's going on at a glance if he or she sees either one. It might make a good little interview question; hand somebody two loops, one using each idiom, and ask 'em what the difference is (proper answer: "nothing", with maybe an addition of "but I habitually use THAT one.")

share|improve this answer
add comment

You could also go with this:

public void doSomething(int input)
{
   while(TransformInSomeWay(input), !ProcessingComplete(input))
   {
      DoSomethingElseTo(input);
   }
}

Or less confusingly:

bool TransformAndCheckCompletion(int &input)
{
   TransformInSomeWay(input);
   return ProcessingComplete(input))
}

public void doSomething(int input)
{
   while(TransformAndCheckCompletion(input))
   {
      DoSomethingElseTo(input);
   }
}
share|improve this answer
    
+1 for the most clear solution so far. –  vikingsteve Apr 8 at 11:33
    
While some people like it, I assert that the loop condition should generally be reserved for conditional tests, rather than statements that do stuff. –  Hurkyl May 25 at 17:07
add comment

If a particular flow of control is naturally expressed as a break statement, it is essentially never a better choice to use other flow control mechanisms to emulate a break statement.

(note that this includes partially unrolling the loop and sliding the loop body down so that the loop begin coincides with the conditional test)

If you can reorganize your loop so that the flow of control is naturally expressed by having a conditional check at the beginning of the loop, great. It might even be worth spending some time thinking about whether that is possible. But it not, don't try to fit a square peg into a round hole.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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