Looping Antipatterns

After programming for a while, looping constructs starts to come naturally that some programmers get stuck when a bug or error occurs in a loop.

Loop Constructs

Recall that any type of loop (for, while, etc) has the following THREE components:

  1. Initializer
  2. Condition
  3. Increment

The Initalizer sets up the condition variable for checking. The Condition controls iteration of the loop; whether or not to perform another iteration. The Increment modifies the condition variable so that the condition will turn false eventually. Note here that the increment does not necessary mean addition of a numeric variable. For example,


BufferedReader in = new BufferedReader(new FileReader("config.txt"));
String data = in.readLine();	// initializer
while (data != null) {		// condition
    // process data
    data = in.readLine();	// increment
}

In this scenario of reading a text file, the increment involves replacing the data variable with new data from the file. Here, the 3 components are labeled and can clearly be seen. There’s no need for you to label all your loops like this, but you should highlight these statements mentally to confirm that you have a valid loop. In a for loop, all 3 components are present in a single line:


for (int i=0;i<depts.length;i++) { // initializer, condition, increment
    // process depts[i]
}

There are other loop forms which do not show the components clearly, as such:


while (true) {
    // do something
    if (condition) break;
    // do something else
}

Though we can sometimes see this format, I discourage frequent use of it because

The anti-patterns

When a looping error occurs – too little/too many iterations, incorrect results, etc – check your construct first, whether you have got all these components. For example when reading the file you may accidentally miss out the increment, so data never becomes null.


BufferedReader in = new BufferedReader(new FileReader("config.txt"));
String data = in.readLine();	// initializer
while (data != null) {		// condition
    // process data, but where is the increment??
}

Another antipattern is the unconditional return.


for (int i=0;i<depts.length;i++) {
    if (depts[i].getName().equals(name)) {
        // do something
        return;
    } else {
        // do something - department not found
        return;
    }
}

You wonder why your loop always only checks the first department. Both the if and else end with a return, so by tracing you would have realized that the returns stops the loop from ever reaching the second iteration. Beware when using such escape techniques within loops, such as breaks and returns, as they complicate the flow of commands in your code. Do a trace to ensure that it doesn’t break out at the wrong time. The correct form of the loop should be:


for (int i=0;i<depts.length;i++) {
    if (depts[i].getName().equals(name)) {
        // do something
        return;
    }
}
// do something - department not found

Also try and remember that if you’re looking for something in a collection, you can only be sure that its not inside the collection AFTER you have traversed the WHOLE collection, not somewhere during the traversal. Therefore such code should only exist AFTER the loop. This will help you identify such bugs more easily.

RSS feed for comments on this post · TrackBack URL

Leave a Comment