Which is better practice - for loop with break or conditional loop?

Question

I'm just curious what peoples' thoughts are on this topic. Let's say I have an Array of Objects, and I want to loop through them to see if the Objects contain certain values, and if so, I want to stop the loop. Which is better practice - a for loop with a break, or a conditional loop?

The pseudo-code in the example I have provided is for argument's sake only (it is also in ActionScript, since that is my primary language of late). Also, I am not looking for best practice ideas on syntax.

for loop with break:

var i:int;

var isBaxterInMilwaukee:Boolean;    

for (i = 0; i < arrayLen; i++)
{
    if (myArray[i]["name"] == "baxter"
         && myArray[i]["location"] == "milwaukee")
    {
        isBaxterInMilwaukee = true;

        barkTwice();

        break;
    }
}

conditional loop:

var i:int;

var isBaxterInMilwaukee:Boolean;    

while (!isBaxterInMilwaukee && i < arrayLen)
{
    if (myArray[i]["name"] == "baxter"
         && myArray[i]["location"] == "milwaukee")
    {
        isBaxterInMilwaukee = true;

        barkTwice();
    }

    i++;
}

Solution 1

In short, you should go with whichever version is the easiest to read and maintain.

In slightly older times, I know breaking out of a loop was considered to be a no-no (on par with a goto statement). Loops were supposed to break on the loop condition and nowhere else. Thus, the while-loop would have been the way to go.

(This is probably a holdover from assembly, where loops are basically a block of code with a go-to-the-beginning-if-true jump statement at the end. Multiple conditional-jump statements in the block make it exceedingly hard to debug; thus they were to be avoided and combined into one at the end.)

I feel this idea seems to be changing a bit today, especially with foreach loops and the managed world; it's really a matter of style now. Break-on-found for-loops have perhaps come to be acceptable to many, save some purists of course. Note that I would still avoid using break in a while-loop, however, as this can muddle the loop condition and make it confusing.

If you'll allow me to use a foreach loop, I consider the code below to be a lot easier to read than its while-loop brother:

bool isBaxterInMilwaukee;    

foreach (var item in myArray)
{
    if (item.name == "baxter" && item.location == "milwaukee")
    {
        isBaxterInMilwaukee = true;    
        barkTwice();
        break;
    }
}

However, as the logic grows in complexity, you may want to consider a prominent comment near the break statement lest it become buried and hard to find.

---

Arguably, this whole thing should be refactored into its own function which doesn't break on found, but actually returns the result (feel free to use the for-loop version instead):

bool isBaxterInMilwaukee(Array myArray)
{      
    foreach (var item in myArray)
    {
        if (item.name == "baxter" && item.location == "milwaukee")
        {
            barkTwice();
            return true;
        }
    }
    return false;
}

As Esko Luontola pointed out, it would probably be best to move the call to barkTwice() outside of this function as the side-effect is not evident from the function's name, nor related to finding Baxter in every case. (Or add a boolean parameter BarkTwiceIfFound and change the line to read if(BarkTwiceIfFound) barkTwice(); to make the side-effect clear.)

---

For the record, you can also do the flag check in the for-loop without a break, but I feel this actually hurts readability because you don't expect an extra condition in a for-loop definition:

var i:int;

var isBaxterInMilwaukee:Boolean;    

for (i = 0; !isBaxterInMilwaukee && i < arrayLen; i++)
{
    if (myArray[i]["name"] == "baxter"
         && myArray[i]["location"] == "milwaukee")
    {
        isBaxterInMilwaukee = true;    
        barkTwice();
    }
}

You can also simulate auto-incrementing mechanics with a while-loop. I don't like this for a few reasons - you have to initialize i to be one less than your real starting value, and depending on how your compiler short-circuits the loop-condition logic, your value of i on exiting the loop may vary. Nevertheless, it is possible and for some people, this can improve readability:

var i:int = -1;

var isBaxterInMilwaukee:Boolean;    

while (!isBaxterInMilwaukee && ++i < arrayLen)
{
    if (myArray[i]["name"] == "baxter"
         && myArray[i]["location"] == "milwaukee")
    {
        isBaxterInMilwaukee = true;
        barkTwice();
    }
}

Solution 2

I've always disliked the use of breaks in code... In this case it doesn't seem to matter but on more involved loops, it can be awfully confusing to another coder reading it. In general, it often results in not understanding how the loop may terminate until the coder spots the nested break deep in the loop. By specifying a flag condition that's checked each iteration of the loop, it makes this much clearer.

This problem would be similar to having return statements that are deep in the body of a method where they're not easily spotted (rather than setting a retVal variable and returning at the end of the method). With a small method, this seems fine, but the bigger it gets, the more confusing this will be.

It's not an efficiency of operation thing, it's a maintainability thing.

Ask your coworkers what's readable and understandable for a particular situation... That's what really matters.

Solution 3

I would say it depends. In this case the loop with the break seems clearer to me.

Solution 4

In a for loop you can also early exit by putting the early exit criteria in the for loop declaration. So for your example you could do it this way:

var i:int;

var isBaxterInMilwaukee:Boolean;    

isBaxterInMilwaukee = false;

for (i = 0; i < arrayLen && !isBaxterInMilwaukee; i++)
{
    if (myArray[i]["name"] == "baxter"
        && myArray[i]["location"] == "milwaukee")
    {
        isBaxterInMilwaukee = true;

        barkTwice();
    }
}

That way you don't need a break, and it's still more readable than a while loop.

Solution 5

There is a conceptual difference between the two. for loops are for iterating over discrete sets and while loops are for repeating statements based on a condition. Other languages add in finally clauses and looping constructs like foreach or until. They tend to have considerably fewer traditional for loops.

In any case, the rule that I use is that for loops iterate and while loops repeat. If you see something like:

while (counter <= end) {
   // do really cool stuff
   ++counter;
}

Then you are probably better off with a for loop since you are iterating. However, loops like:

for (int tryCount=0; tryCount<2; ++tryCount) {
    if (someOperation() == SUCCESS) {
       break;
    }
}

should be written as while loops since they are really repeating something until a condition is true.

The thought of not using break since it is just as evil as goto is pretty nonsensical. How can you justify throwing an exception then? That's just a non-local and non-deterministic goto! By the way, this isn't a rant against exception handling, just an observation.

View more solutions