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?

Answer 1

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() : "";
}

Answer 2

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.