Replace Indexing With Iteration

You are iterating over a collection or array in a ugly old manner.

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

Replace this:

 void process(Collection<Process> processes) {
     for (Iterator<Process> p = processes.iterator(); processes.hasNext();) {
         processes.next().process();
     }
 }

With this:

 void process(Collection<Process> processes) {
     for (Process p : processes) {
         p.process();
     }
 }

Or in Python:

Replace this:

 for i in range(len(processes)):
     processes[i].process()

With this:

 for p in processes:
     p.process()

 Motivation

Iterating over a collection with a traditional for loop is just clutter and prone to introduce errors. If you look carefully at the code, the iterator variable occurs three times in each loop, so you have two chances to get it wrong. Furthermore, the code using the traditional for loop is bigger than it needs to be. Length increase complexity, and complexity makes the code harder to read and understand.

You can make your intention clearer by replacing the for loop with a for-each loop. The for-each construct gets rid of the clutter and the opportunity for error. After the refactoring, the code will be more readable and less prone to host bugs.

Mechanics

Check that you are using the iterator or index variable only to access the elements in the collection or array.
- The for-each loop hides the iterator or index variable while the iteration takes place, so you cannot use these variables to anything else.
Replace the for loop construct with a for-each loop construct.
Compile and test.

Example: Using collections

I start with a simple and fictitious populate method:

 void populate(Collection<Row> rows, Collection<Column> columns) {
      for (Iterator<Row> r = rows.iterator(); r.hasNext(); ) {
           Row row = r.next();
           for (Iterator<Column> c = columns.iterator(); c.hasNext(); ) {
       Column column = c.next();
        matrix.add(new Position(row, column));
           }
      }
 }

The ugly method above has the intention of populating a matrix object with a collection of rows and columns. The iterator variables in both collections are used only to get access to the next element in the chain, so I feel secure to make the refactoring one loop at a time. I start with the inner one:

 void populate(Collection<Row> rows, Collection<Column> columns) {
     for (Iterator<Row> r = rows.iterator(); r.hasNext(); ) {
         Row row = r.next();
         for (Column column : columns) {
             matrix.add(new Position(row, column));
         }
     }
 }

At this point, I compile and test and things should work. Now, I will refactor the outer loop:

 void populate(Collection<Row> rows, Collection<Column> columns)
     for (Row row : rows) {
         for (Column column : columns) {
             matrix.add(new Position(row, column));
         }
     }
 }

I compile and test again and, if no problem occurs, the work has finished. As a final point, make a comparison between both methods, before and after refactoring and you will be much more comfortable.

Example: Using arrays

Suppose you have a method that computes the total amount stored in a price array:

 double totalAmount(double[] prices) {
     double total = 0;

     for (int i = 0; i < prices.length; i++) {
         total += prices[i];
     }

     return total;
 }

Once the for loop temporal index variable (i) is used only to get access to the price elements, we can refactor the for loop to get the following:

 double totalAmount(double[] prices) {
     double total = 0;

     for (double price : prices) {
         total += price;
     }

     return total;
 }

I compile, test and enjoy the resultant code.

-- SantiagoValdarrama?

Acknowledgments:

Thanks to J. B. Rainsberger (Diaspar Software Services) for the names of these refactorings.

This is a clear exposition of how to use iteration instead of indexing; but is it just me that's unconvinced of the why? Coding in Java, I have never been troubled by any of the difficulties ascribed to indexing; coding with VBA collections (as I have to from time to time), I constantly come up against the inherent limitations of iteration. More generally, on the principle of the SimplestThingThatCouldPossiblyWork, how much do iterators, enumerations and so forth add to the functionality of plain-vanilla arrays?

To issue a brief advertisement, indexing over arrays is

indefinitely flexible - forwards, backwards, skipping, set complex exit conditions
inherently type-safe and largely removes the need for casting
easy to debug by printing to the console
(in a modern editor) trivial to code using templates/macros

I have to admit that I'm so convinced of this that I mostly use Java collections features like sorting wrapped in static methods operating on arrays.

Is there a more general point here about what might be called trivial abstraction - syntactic sugar that dilutes the programmer's awareness of what the machine is doing, without conferring any substantial benefit in return?

-- DavidWright

CategoryRefactoring