You are iterating over a collection or array in a ugly old manner.
Replace the iteration code with a for-each loop.
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();
}
}
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 populate 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.
Acknowledgments:
Thanks to J. B. Rainsberger (Diaspar Software Services) for the names of these refactorings.
-
Replace Indexing With Iteration (8 messages)
- Posted by: Santiago Valdarrama
- Posted on: May 24 2005 19:47 EDT
Threaded Messages (8)
- Refactoring? by James Watson on May 25 2005 21:58 EDT
- wrong thread by James Watson on May 25 2005 23:09 EDT
-
Not just a new feature by Santiago Valdarrama on May 27 2005 12:07 EDT
-
Still not a design pattern by James Watson on June 01 2005 11:38 EDT
- Agreed by Dario Souza on February 13 2006 01:44 EST
-
Still not a design pattern by James Watson on June 01 2005 11:38 EDT
-
Not just a new feature by Santiago Valdarrama on May 27 2005 12:07 EDT
- wrong thread by James Watson on May 25 2005 23:09 EDT
- Replace Indexing With Iteration by bura nara on July 13 2005 08:08 EDT
- Replace Indexing With Iteration by James Watson on July 13 2005 08:11 EDT
-
Refactoring?[ Go to top ]
- Posted by: James Watson
- Posted on: May 25 2005 21:58 EDT
- in response to Santiago Valdarrama
The NEW enhanced for loop is syntactical sugar aimed at simplifying code needed for the most common loops. Not using it when it doesn't suffice doesn't really qualify as a refactoring. -
wrong thread[ Go to top ]
- Posted by: James Watson
- Posted on: May 25 2005 23:09 EDT
- in response to James Watson
The above comment should go on the other thread.
On the other hand, I'm not sure how using a new language feature qualifies as a design pattern. -
Not just a new feature[ Go to top ]
- Posted by: Santiago Valdarrama
- Posted on: May 27 2005 00:07 EDT
- in response to James Watson
The aim for this refactoring is not just that the for-each loop is a new language feature in Tiger. The sample is in Java but it can be done in any other language.
The aim is to simplifying the loops. -
Still not a design pattern[ Go to top ]
- Posted by: James Watson
- Posted on: June 01 2005 11:38 EDT
- in response to Santiago Valdarrama
Not all languages have this syntax.
Regardless, it still doesn't qualify as a design pattern. -
Agreed[ Go to top ]
- Posted by: Dario Souza
- Posted on: February 13 2006 01:44 EST
- in response to James Watson
You're right. Design patterns are more specific to structural abstractions rather than naming specific language parser mechanism.
Nothing can be more of a syntatic sugar than this is. -
Agreed[ Go to top ]
- Posted by: Hue Cheng
- Posted on: April 24 2006 02:08 EDT
- in response to Dario Souza
-
Replace Indexing With Iteration[ Go to top ]
- Posted by: bura nara
- Posted on: July 13 2005 08:08 EDT
- in response to Santiago Valdarrama
for (double price : prices) {} giving error as Syntax error on token ":", ";" expected.
I am using j2sdk1.4.1_01 at present. Which version will be supported by FOR each loop.
reg,
HAM -
Replace Indexing With Iteration[ Go to top ]
- Posted by: James Watson
- Posted on: July 13 2005 08:11 EDT
- in response to bura nara
for (double price : prices) {} giving error as Syntax error on token ":", ";" expected.I am using j2sdk1.4.1_01 at present. Which version will be supported by FOR each loop.reg,HAM
JDK 1.5 and greater.