CODEWITHSUNDEEP
javaalgorithmcollectionslist
Arvodan
Arvodan

Reputation: 1205

Java: How to remove elements from a list while iterating over/adding to it

This question is a more special case of the problem described (and solved) in this question.

I have two methods, stopAndRemove(ServerObject server) and a close() method. The later should close all servers and remove them from the server list. The list is defined as

List<ServerObject> server.

I do not want to have almost the same code from stopAndRemove in closeCurrentlyOpen, so I want to do something like:

public void closeCurrentlyOpen() {
   for(ServerObject server : this.servers) {
       stopAndRemove(server)
   }
}

This won't work, as this will cause a ConcurrentModificationException. I tried to make a copy of the list 

List<ServerObject> copyList = new ArrayList<ServerObject>(this.servers);

and use that as the list for the foreach-loop. But then it might be possible that an other thread appends a Server to the servers list while I am iterating over copyList but closeCurrentlyOpen is supposed to result in an emtpy list. As the addServerToList method is synchronized to the servers-list, doing this

public void closeCurrentlyOpen() {
   synchronized(this.servers) {
     for(ServerObject server : this.servers) {
        stopAndRemove(server)
     }
    }
}

will solve the problem with modifications. But then I can not synchronize the code in the stopAndRemove method which is necessary if it is directly called.

I seems to me that the design of this three methods probably needs a workover. Ideas anybody?

Upvotes: 17

Views: 61889

Answers (9)

Raskolnikov
Raskolnikov

Reputation: 159

Similar to firebird84. But u can use removeAll(Collection c) api

for(String exitingPermission : existingPermissions){                
    //remove all permissions for the screen and add the new ones
    if(exitingPermission.split("_")[0].equals(screen)){
        removePermissions.add(exitingPermission);
    }
 }
existingPermissions.removeAll(removePermissions);

Upvotes: 1

sparkyspider
sparkyspider

Reputation: 13519

... removing files that aren't XML from a directory list...

List<File> files = Arrays.asList(dir.listFiles());

Iterator<File> i = files.iterator();

while (i.hasNext()) {
    File file = i.next();
    if (!file.getName().endsWith(".xml")) {
        i.remove();
    }
}

Upvotes: 1

Neeme Praks
Neeme Praks

Reputation: 9150

Answering to the title of the question, not the specific details of the given example. In fact, this solution is not even appropriate in the given situation (refactoring is appropriate, as suggested by others).

However, it seems that many java programmers are not aware of CopyOnWriteArrayList (part of JDK since 1.5) and are trying to roll their own solutions to the same problem (copy list before iterating).

Upvotes: 1

Bob Gettys
Bob Gettys

Reputation: 1202

Perhaps this is the wrong way to do it, but I always create a removal collection, which contains indexes or references to the objects that need to be removed. I then iterate over that collection and remove those indexes/objects from the original collection. Probably not the most efficient but it got the job done.

Instead of 

for(Collection things : thing)  
    things.remove(thing)

I use

Collection toRemove = new LinkedList();
for(things : thing)
    toRemove.add(thing);

for(toRemove : thing)
    things.remove(thing)

Upvotes: 14

Alex Miller
Alex Miller

Reputation: 70211

You might find this article about ConcurrentModificationException has some advice in this area.

Upvotes: 3

amit
amit

Reputation: 10872

You should get an iterator and remove using it. You are getting the exception because iterators are fail-fast in java.

Upvotes: 1

starblue
starblue

Reputation: 56772

Split off a method stop() from stopAndRemove(). Then write the loop with an explicit iterator, do the stop and then iterator.remove().

"and" in a method name is a code smell.

Upvotes: 17

Pesto
Pesto

Reputation: 23880

Refactor out all the ServerObject stopping code from stopAndRemove into a private stopServer method, and then do the removal separately in stopAndRemove and closeCurrentlyOpen. Then you can use a ListIterator to remove them (or just stop them all in a for loop and clear the list at the end).

Upvotes: 2

Eric Petroelje
Eric Petroelje

Reputation: 60498

When I've done this before, I always used the "old school" LinkedList collection, an Iterator, and the Iterator.remove() method to remove the current item.

Upvotes: 4

Related Questions