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.

Answer 1

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).

Answer 2

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.