Best Practice - Loop Exit Via Iterator Modification

Question

I had an interesting discussion with my boss today regarding exiting a loop before the terminal condition is met. He had a construct in some of his VB6 code that looked something like this:

Dim Termination As Int
Termination = 10

For Iterator1 = 1 To Termination
    If Iterator1 = 5 Then
        Iterator1 = Termination
    End If
Next

I questioned this implementation, contending that an Exit For was the proper way to exit a loop prematurely based on a condition, like this:

For Iterator2 = 1 To 10
    If Iterator2 = 5 Then
        Exit For
    End If
Next

I noted that in practice these probably both evaluate to the same machine language JMP opcode. My boss asserted that his implementation may be more efficient since it would have only one JMP, the one at the end of the loop, instead of two, one at the end and one in the middle for the Exit For statement.

I prefer the second implementation for readability and semantics. Also, in the latest versions of Visual Studio for .Net if you add lines after the Exit For statement you'll be warned that there are unreachable lines in your code. However, you don't get this warning in VB6, which is where the debate started.

So what's the best practice here? Is the first example better in VB6 or is the second example better in all situations? What are the advantages / disadvantages of each approach.

Edit: Removed magic number from code example to enhance question clarity.

@Dan Pichelman - I agree, that's the purpose of that language construct. However, your answer doesn't indicate whether it's the most efficient way to terminate a loop. Also, I'm looking for the advantages and disadvantages of either approach. I'll edit the question to be more precise. — Bob Mc, Oct 07 '13 at 20:52
I would not expect a jump to be more expensive than an assignment, a jump, another conditional, followed by a jump. But that's just me. — whatsisname, Oct 07 '13 at 20:53
@whatsisname Yet, oddly enough, that unintuative option can in fact be the case, given the way CPUs are constantly working ahead of the current execution based on the expected subsequent operations. Branching in an expected way results in branches that are much cheaper than branching in an unexpected way. — Servy, Oct 07 '13 at 20:56
@Servy: while that is often true, you are usually best letting the compiler make those decisions, and I doubt the VB6 runtime is that sophisticated. — whatsisname, Oct 07 '13 at 21:06
@whatsisname Sure. Largely I'd expect there to be no measurable difference. I was just providing an interesting (but also largely irrelevant) fact. — Servy, Oct 07 '13 at 21:07
Just to point out that when someone adds some code AFTER the "End If" then your bosses code could very well now be broken. It might not matter in such a simple case because it's easy to see, but most simple things become less simple as time goes on. — Dunk, Oct 07 '13 at 22:06
These two approaches are NOT equivalent. Your boss is an idiot. — James, Oct 07 '13 at 22:30
you and your boss got it wrong, boss' code will run the rest of the loop body test the condition and then break out, `Exit For` will jump out of the loop immediately. however any difference here (besides the semantics) is so small that you won't be able to detect it — ratchet freak, Oct 08 '13 at 08:21

score 4 · Accepted Answer · answered Oct 07 '13 at 21:06

Performance shouldn't be your first priority. Of higher importance should generally be code readability and maintainability. Exit For states, in code, exactly what you want to do, semantically. It's blatantly obvious to the reader exactly what the code is doing.

When setting the loop variable it's a lot less clear. I need to spend time trying to figure out what's going on; is the loop variable set to the end, ahead one, the start, somewhere in the middle, or what? That's time I shouldn't need to be spending.

Beyond that, you're also now repeating the constant 10 in that loop. What if someone goes to change the loop variable to go to 15 instead. They may not bother to look through the entire body of the loop and just expect it to work fine going up to 15 instead. You now have a very evil bug. When I come along to debug the code I may not know if someone intended the loop variable to be moved to another internal location, or if it really should be the end of the loop.

Another key difference is that End For ends the current iteration of the loop right now. Setting the loop variable means that the current iteration ends and then the loop stops. If there is more code in the current iteration of the loop then that's different. It's only the same if the code is at the end of the loop.

As for the performance differences, I wouldn't expect there to be any particularly significant difference. In the event there is a difference at all, it's almost certainly going to be very, very, very tiny, and not enough to matter. If, by some surprising series of events you've managed to determine that this path of this loop is executed so frequently, and in a context where performance is so vital, that this change results in a noticeable and essential performance difference through your in depth performance tests and profiling, then consider using the less readable version (possibly with supporting comments).

I like this answer, and not just because it agrees with my assertions, but because your reasoning is sound. Btw, the hard-coded "magic number" was just for purposes of illustration. In the actual code the loop terminator value was a variable that was set earlier in the code, so your concern, while correct and valid, was not applicable in the original discussion. — Bob Mc, Oct 07 '13 at 21:40
@BobMc Even so, someone can change the variable that the loop goes to without realizing that they need to change the variable used inside of the loop as well; it's the same fundamental problem. — Servy, Oct 08 '13 at 00:21

score 0 · Answer 2 · answered Oct 07 '13 at 20:32

Any time you exit a loop before meeting the condition you set at the top of the loop, you're causing some unexpected behaviour. That's generally not a good thing (it's the same problem people have with the oft maligned GOTO statement.) If you manually manipulate the data to meet the top condition, that's almost worse.

If I want to loop until I break out of it manually, I'd use some version of "while(true){}" and then explicitly break out with an exit() statement. That lets the reader know that the top condition of the loop doesn't govern when the loop stops, and doesn't leave them scratching their heads when the code exits at 10, after only looping 5 times.

Exiting a loop via an setting an arbitrary value to the iterator is the sort of thing I'd expect to see in VB6 though. That's practically a metaphor for the whole language.

Thanks for the reply. What about a situation where you know that you have to loop through ten items, but the first occurrence of a certain data point in one of those items makes the rest of the loop irrelevant? You can achieve this with a counter variable in your `while(true)` loop but isn't that what an iterated `For` loop is intended to simplify? — Bob Mc, Oct 07 '13 at 20:57
"Any time you exit a loop before meeting the condition you set at the top of the loop, you're causing some unexpected behaviour" - not really — James, Oct 07 '13 at 22:34

Best Practice - Loop Exit Via Iterator Modification

2 Answers2