Replace Iteration With Indexing

You are iterating over a collection or array using the enhanced for loop, and you need to have access to the iterator or index variable in order to make some operation with it.

Replace the for-each construct with a classic for loop. Possible only since java 1.5 aka 'Tiger'.

Replace this:

 int locate(int[] values, int value) {
     int index = 0;
     for(int v : values) {
         if (v == value) {
             return index;
         }
         index++;
     }
     return -1;
 }

With this:

 int locate(int[] values, int value) {
     for(int index = 0; index < values.length; index++) {
         if (values[index] == value) {
             return index;
         }
     }
     return -1;
 }

Motivation

In some situations, the use of the enhanced for loop, also called for-each, is not suitable for iterating over collections and arrays. In the example above, the code shows a method that finds a value in an array and returns its position. The for-each construct hides the index variable, so to use it, we need to create a temporary variable and increment it on each iteration to later return the value position. This process is automatically carried out by a traditional for loop, so it's better for this kind of problem.

Another situation when you should decline to use iteration is when you need to replace elements in a collection or array as you traverse it. (Although for collections, you should accomplish this with a traditional for loop and an iterator, and use the iterator's "remove" and list iterator's "set" methods to modify the collection.) Also, the for-each construct is not usable for loops that must iterate over multiple collections in parallel.

Using the wrong loop may cause behavioral problems or, in the best case, decrease code clarity.

Mechanics

Check that you really need to have access to the iterator or index variable.
- If not, you should not truncate the for-each construct.
Replace the for-each loop construct with a for loop construct.
Compile and test.

Example

Lets look at this example where we want to expunge all empty strings from a linked list:

 void expunge(LinkedList<String> list) {
     int index = 0;
     for(String value : list) {
         if (value.equals("")) {
             list.remove(index);
         }
         index++;
     }
 }

The code above seems to be right, but it isn't. Run it and you'll get a ConcurrentModificationException?. The problem is that the for-each construct uses a fail-fast iterator to move over collections, so using a for-each loop you can't remove elements while iterating over any collection without getting a ConcurrentModificationException?.

To get the proper behavior, and simplify the method, replace the for-each loop with a traditional for loop over an iterator:

 void expunge(LinkedList<String> list) {
     for(Iterator<String> iterator = list.iterator(); iterator.hasNext();) {
         if (iterator.next().equals("")) {
             iterator.remove();
         }
     }
 }

I compile and test, and no exception is thrown. Looking at the code, we don't need the temporary index variable anymore, because we are using an iterator embedded in the for loop construct. The code works fine, and it shows its intention in a clearer manner.

-- SantiagoValdarrama?