Using iterators would allow for some more idiomatic work for removing/adding the listener. It is cumbersome the way it is written.

[flsobral]

Using iterators would allow for some more idiomatic work for removing/adding the listener. It is cumbersome the way it is written.

Instead of

  public void removeUpdateListener(UpdateListener listener) {
    int idx = updateListeners.size() - 1;
    while (idx >= 0 && updateListeners.get(idx).get() != listener) {
      idx--;
    }
    if (idx >= 0) {
      updateListeners.remove(idx);
    }
  }

one would have

  public void removeUpdateListener(UpdateListener listener) {
    Iterator<WeakReference<UpdateListener>> it = updateListeners.iterator();
    while (it.hasNext()) {
      WeakReference<UpdateListener> el = it.next();
      if (el.get() == listener) {
        it.remove();
        return;
      }
    }
  }

Also, when iterating and removing from the tail while triggering those listeners, with iterators, should be something like:

      ListIterator<WeakReference<UpdateListener>> it = updateListeners.listIterator(updateListeners.size());
      while (it.hasPrevious()) {
        UpdateListener ul = it.previous().get();
        if (ul != null) ul.updateListenerTriggered(elapsedMilliseconds);
        else it.remove();
      }

if wished for a tail first remotion. But also

      Iterator<WeakReference<UpdateListener>> it = updateListeners.iterator();
      while (it.hasNext()) {
        UpdateListener ul = it.next().get();
        if (ul != null) ul.updateListenerTriggered(elapsedMilliseconds);
        else it.remove();
      }

Note that both alternatives aren't great uses for ArrayLists, as removing from anything but the tail can be burdensome because of array shifts. But I think that it is a little more idiomatic Java than keeping those pesky indexes.

As I like a more idiomatic approach, however, one should notice that there may be more calls to virtual methods in my suggestions than in the current PR. And also as it is expected that this list has more volatile members, may using LinkedList is a better approach to avoid array shifts? Needs some more performance research to a final word, maybe.

Originally posted by @jeffque in #357 (comment)