Should listeners be able to remove listeners?

Question

Many classes use code similar to the following to fire listeners.

private List<Listener> listeners = new ArrayList<Listener>();

public void fireListener() {
  for(Listener l : listeners) l.someMethod();
}

This works fine until the listener tries to add/remove a listener. This modification from inside the list causes a ConcurrentModificationException. Should we handle this case or should modifying listeners be invalid? What would be the best way to handle add/remove of listeners?

Update:
Here is one possible solution.

public void fireListener() {
  for(Listener l : listeners.toArray(new Listener[listeners.size()])) {
    if(!listeners.contains(l)) continue;
    l.someMethod();
  }
}

Answer 1

There are three cases:

1. You don't want to allow the modification of the listeners collection during listeners execution:
   A ConcurrentModificationException would be appropriate in this case.

2. You want to allow modification of listeners, but the changes are not to be reflected in the current run:
   You have to make sure a modification of listeners does not have impact on the current run. A CopyOnWriteArrayList does the trick. Before using it read the API, there are some pitfalls.
   Another solution would be copying the list before iterating through it.

3. You want changes to listeners reflect within the current run:
   The most tricky case. Using for-each loops and iterators won't work here (as you have noticed ;-)). You have to keep track of changes yourself.
   A possbile solution would be to store listeners in an ArrayList and go through that list using a standard for loop:

   for (int i =0; i < listeners.size(); i++) { 
       Listener l = listeners.get(i);
       if (l == null) 
           continue;
       l.handleEvent();
   }  
   

   Removing listeners would set the element at its place in the array to null. New listeners are added to the end and therefore will run in the current execution.
   Notice that this solution is just an example and not threadsafe! Maybe some maintainance is necessary to remove null elements sometimes to keep the list from growing too big.

It is up to you to decide what is needed.

My personal favourite would be the second one. It allows modifications while execution but does not change the current run's behaviour which can cause unexpected results.