Trouble iteration over a list and removing items

[daschicken]

Hi guys,

ive written the following method.
its supposed to delete objects in a LinkedList.
These objects implicit Connections to other objects.
if one object is deleted all the connections in the other objects should be deleted as well.

all in all this code should do what I want. the only problem is that i get an ConcurrentModif icationExceptio n running it.

in the API it says

This exception may be thrown by methods that have detected concurrent modification of an object when such modification is not permissible.

unfortunately I cant figure out the problem

would be great if anybody can help.!

Thanks very much

  public void removeEntry(AddressBookEntry en) throws AddressBookException{
        LinkedList<Connection> temp;
        temp = en.getConnections();        
        if(!temp.isEmpty()){
            for(Connection con : temp){
                en.deleteConnection(con);
            }
        }
        entries.remove(en); 
    }

[BigDaddyLH]

First, just for fun, that could have been written more succinctly as:

[CODE=Java]public void removeEntry(Add ressBookEntry en) throws AddressBookExce ption{
for(Connection con : en.getConnectio ns()){
en.deleteConnec tion(con);
}
entries.remove( en);
}[/CODE]
(why test for empty when the loop does that for you.)

Second, you forgot to mention which line throws the exception!

[daschicken]

the loop itself
line 5 :)

thanks for the tip

[BigDaddyLH]

the loop itself
line 5 :)

thanks for the tip

I don't have all your code in front of me. Does deleteConnectio n remove the Connection from the list, too?

[daschicken]

    public void deleteConnection(Connection c) throws AddressBookException{
        AddressBookEntry en;
        if( c.o2 == this){
            en = (AddressBookEntry) c.o1;
        }
        else{
            en = (AddressBookEntry) c.o2;
        } 
        connections.remove(c);
        en.connections.remove(c);       
    }

"connection s" is an attribute (LinkedList) of the AddressBookEntr y class

[BigDaddyLH]

    public void deleteConnection(Connection c) throws AddressBookException{
        AddressBookEntry en;
        if( c.o2 == this){
            en = (AddressBookEntry) c.o1;
        }
        else{
            en = (AddressBookEntry) c.o2;
        } 
        connections.remove(c);
        en.connections.remove(c);       
    }

"connection s" is an attribute (LinkedList) of the AddressBookEntr y class

That's your error then. You are mutating the list as you iterate over it.

[daschicken]

so thats not allowed?
do you know a way to avoid it?

maybe use a usual for or a while loop?

[BigDaddyLH]

The code you posted (from class X?) seems to be doing work that AddressBookEntr y should be doing:

[CODE=Java]for(Connection con : en.getConnectio ns()){
en.deleteConnec tion(con);
}[/CODE]

Perhaps AddressBookEntr y can avoid exposing a getConnections method and define instead a deleteAllConnec tions()?

Then in that method, use the proper way to remove elements as you iterate: the remove method of Iterator:

[CODE=Java]for(Iterator<Co nnection> it = getConnections( ); it.hasMore(); ) {
Connection con = it.next();
....
if (condition)
it.remove();
}[/CODE]

[daschicken]

hi,

your tip gave me the right direction, thanks very much for that!

in case your interested I post my working code. I'm not sure if its coded very nice but as im relatively new to java I'm glad its working at all :D.

    public void deleteAllConnections() throws AddressBookException{  
        AddressBookEntry en;
        for(Iterator<Connection> it = this.getConnections().listIterator(); it.hasNext();) {
          Connection con = it.next();
      // define second connected object  
        if( con.o2 == this) en = (AddressBookEntry) con.o1;
        else  en = (AddressBookEntry) con.o2;
      // remove connection from both elements  
        en.connections.remove(con);
        it.remove();
      }
    }

[BigDaddyLH]

An alternative to it.remove() would be to call clear() on the list after the loop. Simpler and faster:

[CODE=Java]public void deleteAllConnec tions() throws AddressBookExce ption{
for(Connection con : getConnections( )) {
AddressBookEntr y en = (AddressBookEnt ry) (con.o2 == this? con.o1 : con.o2);
en.connections. remove(con);
}
getConnections( ).clear();
}[/CODE]

[daschicken]

very nice :) as I notice I have to practice a lot :)

thanks