[java] Calling remove in foreach loop in Java

In Java, is it legal to call remove on a collection when iterating through the collection using a foreach loop? For instance:

List<String> names = ....
for (String name : names) {
   // Do something
   names.remove(name).
}

As an addendum, is it legal to remove items that have not been iterated over yet? For instance,

//Assume that the names list as duplicate entries
List<String> names = ....
for (String name : names) {
    // Do something
    while (names.remove(name));
}

Use

.remove() of Interator or

Use

CopyOnWriteArrayList

Yes you can use the for-each loop, To do that you have to maintain a separate list to hold removing items and then remove that list from names list using removeAll() method,

List<String> names = ....

// introduce a separate list to hold removing items
List<String> toRemove= new ArrayList<String>();

for (String name : names) {
   // Do something: perform conditional checks
   toRemove.add(name);
}    
names.removeAll(toRemove);

// now names list holds expected values

I didn't know about iterators, however here's what I was doing until today to remove elements from a list inside a loop:

List<String> names = .... 
for (i=names.size()-1;i>=0;i--) {    
    // Do something    
    names.remove(i);
} 

This is always working, and could be used in other languages or structs not supporting iterators.

1. Try this 2. and change the condition to "WINTER" and you will wonder:

public static void main(String[] args) {
  Season.add("Frühling");
  Season.add("Sommer");
  Season.add("Herbst");
  Season.add("WINTER");
  for (String s : Season) {
   if(!s.equals("Sommer")) {
    System.out.println(s);
    continue;
   }
   Season.remove("Frühling");
  }
 }

Those saying that you can't safely remove an item from a collection except through the Iterator aren't quite correct, you can do it safely using one of the concurrent collections such as ConcurrentHashMap.

for (String name : new ArrayList<String>(names)) {
    // Do something
    names.remove(nameToRemove);
}

You clone the list names and iterate through the clone while you remove from the original list. A bit cleaner than the top answer.

The java design of the "enhanced for loop" was to not expose the iterator to code, but the only way to safely remove an item is to access the iterator. So in this case you have to do it old school:

 for(Iterator<String> i = names.iterator(); i.hasNext();) {
       String name = i.next();
       //Do Something
       i.remove();
 }

If in the real code the enhanced for loop is really worth it, then you could add the items to a temporary collection and call removeAll on the list after the loop.

EDIT (re addendum): No, changing the list in any way outside the iterator.remove() method while iterating will cause problems. The only way around this is to use a CopyOnWriteArrayList, but that is really intended for concurrency issues.

The cheapest (in terms of lines of code) way to remove duplicates is to dump the list into a LinkedHashSet (and then back into a List if you need). This preserves insertion order while removing duplicates.

You don't want to do that. It can cause undefined behavior depending on the collection. You want to use an Iterator directly. Although the for each construct is syntactic sugar and is really using an iterator, it hides it from your code so you can't access it to call Iterator.remove.

The behavior of an iterator is unspecified if the underlying collection is modified while the iteration is in progress in any way other than by calling this method.

Instead write your code:

List<String> names = ....
Iterator<String> it = names.iterator();
while (it.hasNext()) {

    String name = it.next();
    // Do something
    it.remove();
}

Note that the code calls Iterator.remove, not List.remove.

Addendum:

Even if you are removing an element that has not been iterated over yet, you still don't want to modify the collection and then use the Iterator. It might modify the collection in a way that is surprising and affects future operations on the Iterator.

Make sure this is not code smell. Is it possible to reverse the logic and be 'inclusive' rather than 'exclusive'?

List<String> names = ....
List<String> reducedNames = ....
for (String name : names) {
   // Do something
   if (conditionToIncludeMet)
       reducedNames.add(name);
}
return reducedNames;

The situation that led me to this page involved old code that looped through a List using indecies to remove elements from the List. I wanted to refactor it to use the foreach style.

It looped through an entire list of elements to verify which ones the user had permission to access, and removed the ones that didn't have permission from the list.

List<Service> services = ...
for (int i=0; i<services.size(); i++) {
    if (!isServicePermitted(user, services.get(i)))
         services.remove(i);
}

To reverse this and not use the remove:

List<Service> services = ...
List<Service> permittedServices = ...
for (Service service:services) {
    if (isServicePermitted(user, service))
         permittedServices.add(service);
}
return permittedServices;

When would "remove" be preferred? One consideration is if gien a large list or expensive "add", combined with only a few removed compared to the list size. It might be more efficient to only do a few removes rather than a great many adds. But in my case the situation did not merit such an optimization.

It's better to use an Iterator when you want to remove element from a list

because the source code of remove is

if (numMoved > 0)
    System.arraycopy(elementData, index+1, elementData, index,
             numMoved);
elementData[--size] = null;

so ,if you remove an element from the list, the list will be restructure ,the other element's index will be changed, this can result something that you want to happened.