Why does this for loop over an ArrayList generate a warning that "the for statement does not loop" in IntelliJ

Question

I have an ArrayList that contains instances of the Staff class. When I write the following code I am told by IntelliJ that the 'for statement does not loop':

public String getTypist() {
    String tempTy = "";
    for (Staff g : staffList) {
        if (g.getStaffID().contains("TY") && g.isAvailable()){
            tempTy = g.getStaffID();
        }
        staffList.remove(g);
        staffWorking.add(g);
        break;
    }

    return tempTy;
}

I'm really confused as I thought that this was the way to properly use a for loop on an ArrayList. What am I doing incorrectly with my for loop?

You call `break;` during the first iteration, so the loop will never iterate. Also, you can't modify an `ArrayList` while you're iterating it. — nickb, Dec 23 '16 at 16:58
Go to your IDE, start this in debugging and set a breakpoint at the start of the method. Then step it through. You will immediately see your error. You'll also have to use an Iterator to remove elements while iterating. — Fildor, Dec 23 '16 at 17:07

Emily Mabrey · Accepted Answer · 2016-12-23T20:19:02.550

Your for loop contains a break statement which always executes no matter what happens before in the loop, so after the very first looping occurs, the break happens, and nothing else is looped afterwards. This basically makes it as though there was no for loop at all, as executing code one time is the default way a series of statements are executed. Correcting this involves making sure the break executes only some of the loops (specifically, making sure it only executes on the loop you want to be the last loop). Making that correction with the addition of some other fixes you would get code similar to this:

public String getTypist() {
    for (Staff s : staffList) {
        if (s.getStaffID().contains("TY") && s.isAvailable()){
            staffList.remove(s);
            staffWorking.add(s);
            return s.getStaffID();
        }
    }

    return "";
}

However, there is an alternative solution that would allow you to avoid iterating over the ArrayList at all. You can replace that code with this code and it will work without any for loop because it uses the methods of the ArrayList itself to accomplish the task:

public String getTypist() {
    ArrayList<Staff> staffWorking = new ArrayList<>(staffList);        
    staffWorking.removeIf(staff -> !(staff.isAvailable() && staff.getStaffID().contains("TY")));

    staffList.removeAll(staffWorking);

    Optional<Staff> typist = staffWorking.stream().findFirst();
    if(typist.isPresent()){
        return typist.getStaffID();
    }else{
        return "";
    }
}

Though even that could be simplified and improved to this (this code supports concurrent filtering so on multi-processor systems it will be much faster):

private static final Predicate<Staff> isATypistWorker = 
    staff -> staff.isAvailable() && staff.getStaffID().contains("TY");

public String getTypist() {
    ArrayList<Staff> typistWorkers = staffList.stream()
         .parallel()
         .filter(isATypistWorker)
         .distinct()
         .collect(Collectors.toCollection(ArrayList::new));

    staffList.removeAll(typistWorkers);
    staffWorkers.addAll(typistWorkers);

    Optional<Staff> typist = typistWorkers.stream().findFirst();

    return typist.isPresent() ? typist.getStaffID() : "";
}

score 0 · Answer 2 · answered Dec 23 '16 at 16:58

0

You aren't looping because you always break after the first iteration of the loop. I think you need braces around your if statement.

public String getTypist() {
    String tempTy = "";

    for (Staff g : staffList) {
        if (g.getStaffID().contains("TY") && g.isAvailable()) {
            tempTy = g.getStaffID();
            staffList.remove(g);
            staffWorking.add(g);
            break;
        }
    }

    return tempTy;
}

Also the code I posted won't work because you can't remove from an ArrayList while looping through it. But that is a different issue.

answered Dec 23 '16 at 16:58

Adam

2,214
1
15
26

I used the break as i was trying to find the first instance that met the criteria and then stop...i guess a for loop is not the correct statement? – Patrick Dec 23 '16 at 17:02
The difference between the code I posted and the code you posted is I have brackets after my `if` statement to contain the rest of the logic in the loop. If you don't understand the difference, read [this](http://stackoverflow.com/a/15786982/2464657). Also a `for` loop won't work for you because you cannot remove from an `ArrayList` while iterating over it. If you don't understand that, read [this](http://stackoverflow.com/questions/10431981/remove-elements-from-collection-while-iterating). – Adam Dec 23 '16 at 17:06

Why does this for loop over an ArrayList generate a warning that "the for statement does not loop" in IntelliJ

2 Answers2