while(true) and loop-breaking - anti-pattern?
- by KeithS
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?
