Refactor foreach to for loop

Question

I have a foreach loop in Java (simplified version here)

List<String> names = getNames();
for(String name:names) {
    doSomething(name);
}

Is there an automated way to refactor this to a traditional for loop?

I know how to do it manually

List<String> names = getNames();
for(int i=0; i<names.size(); i++) {
    String name = names.get(i);
    doSomething(name);
}

As you can see, there is a bit of typing needed in the for statement itself as well as introducing the variable name again and assign it the value names.get(i). In total, the manual edit is too error-prone for me.

Why do I want to do this? I have to fix a bug and the fix is to start at index 1 instead of index 0 and end at index n-1 instead of the end (unfortunately I can't fix the input right away, I need to wait for a library update if that's recognized as a bug).

What have I tried? I right-clicked on the for keyword and clicked on "Refactor", but as far as I can get from the context menu entries, nothing in there would do the work for me.

Why do I think this could theoretically work? Because similar functionality exists in Resharper for Visual Studio (C#).

FYI: I'm using Eclipse Luna SR 2 (4.4.2)

Durkin

Is it possible to introduce a new method instead of 'getNames' which returns the list without first and last items ? – Meroblastic

@DevBlanked: of course that would technically be possible, but doesn't that mean I manually write a loop that copies everything except the first and the last item? – Durkin

not really, you could remove the first & last items giving their indexes, docs.oracle.com/javase/7/docs/api/java/util/…. – Meroblastic

Answer 1

Mouseover the for statement, right-click, Quick fix (Ctrl+1), convert to indexed loop.

Should work!

Recalcitrate

Then call it the ctrl+1 action… btw, you don’t need to highlight the for statement, having the cursor besides it is enough. – Groundwork

@Thomas, to be precise Eclipse offers "quick fixes" for correcting problems (errors/warnings), and "quick assists" for other typical modifications. Loop conversion is a "quick assist". Only for convenience are both actions bound to ctrl-1. – Rugen

Answer 2

List<String> names = getNames();
names = names.subList(1, names.size() - 1);
for(String name : names) {
    doSomething(name);
}

Of course, you could put that into a reusable method if you need to do it several times:

public static List<String> fixList(List<String> names) {
    return names.subList(1, names.size() - 1);
}

and then use it as

List<String> names = fixList(getNames());
for(String name : names) {
    doSomething(name);
}

Gas

Ok, not exactly what I asked for, but definitely nice because if I get the library update, I just have to remove one line of code (doing the fix). – Durkin

This solves the problem you would've had if you got the answer you where looking for :) I would change fixList to trimList and add arguments beginIndex and endIndex making the function more reusable. – Gowrie

Answer 3

In my eclipse (Kepler RC2) it works to select the for keyword and either use the quick fix from the context menu or hit CTRL+1 for the shortcut. Eclipse then offers me "Convert to indexed 'for' loop" or "Convert to Iterator-based 'for' loop".

Cressida

Answer 4

You can use either:

names = names.subList(1, names.size()-1);
for (String name : names) {
   doSomething(name);
}

or in manually:

for (int i = 1; i < names.size()-1; i++) {
   String name = names.get(i);
    doSomething(name);
}

But the first one I prefer to use.

Torture

Answer 5

Be careful with this refactoring.

The reason is that for a traditional linked list, your second for loop formulation is O(N * N) since you're having to traverse the linked list in order to evaluate names.get(i);. That could become expensive.

Do consider performance implications when moving from for(String name:names) {. There may well be a better way of fixing your immediate bug, and retaining the current "Big O".

for(String name : names.subList(1, names.size() - 1)) {

is one such way (Acknowledge @JB Nizet).

Mcelroy

While correct in principle, no sane programmer uses LinkedList anyway. It’s advantage of O(1) insertion cost pays off only at such big lists that its disadvantage of insanely higher per-element memory consumption makes it unusable. – Groundwork

Answer 6

Don't go for indexed iteration! This does not perform well with all implementations of List.

It is much better to go for an Iterator and let the JIT optimize the Iterator away if this code is hot.

So either write this:

List<String> names = getNames();
for (String name : names.subList(1, names.size() - 1)) {
    doSomething(name);
}

Or that (one allocation less):

Iterator<String> it = getNames().iterator();
it.next(); // You seem to be sure there is more than one element in the list
while (it.hasNext()) {
    String name = it.next();
    doSomething(name);
}

Literacy

Answer 7

In my case, I need to transform the foreach to use an index . This is my two cents

Integer index=0;
                  
      for (final String OneCell :CellList){
     // Your code use the index
             index=index+1;
     }

Saberhagen

Answer 8

When using java 8 you could use the stream api

names.stream().skip(1).reverse().skip(1).reverse().foreach(
    name -> do something(name)
);

Something like this...

Spirograph

There is no reverse() method on Stream. – Gas

Besides that this is not what the questioner asked for; there is no reverse method on Stream and forEach does not respect the source order, therefore forEachOrdered is needed. – Groundwork