Java: implementation of notification provider vs. hashCode-driven Map

Question

I have implemented abstract generic provider for notification bunch of generic listeners E, descendants have to override notifyListener(E) with specific notification code. For backing list of listeners I choose WeakHashMap<K,V>. Listeners must be held as weak references:

abstract public class NotificationProvider<E> {

    private Map<E, Object> listeners = new WeakHashMap<E, Object>();

    public addListener(E listener) {
        listeners.put(listener, null);
    }

    public void notifyListeners() {
        for (E listener: listeners.keySet())
            notifyListener(listener);
    }

    abstract protected void notifyListener(E listener);
}

Typical use:

    NotificationProvider<MyListener> provider;
    provider = new NotificationProvider<MyListener>() {
        @Override
        protected void notifyListener(MyListener listener) {
            listener.myNotification();
        }
    }
    provider.addListener(myListener1);
    provider.addListener(myListener2);
    provider.notifyListeners();

Everything works well, but when I need AbstractList descendant class as listener, backing WeakHashMap accepts only one listener instance! It's clear -- methods hashCode() and equals() on listeners return same value for all of instances (empty lists), so WeakHashMap.put only replace previously added listener.

    public class MyList extends AbstractList<MyItem> {
        // some implementation
    }
    NotificationProvider<MyList> provider;
    provider = new NotificationProvider<MyList>() {
        @Override
        protected void notifyListener(MyList listener) {
            // some implementation
        }
    }
    MyList list1 = new MyList();
    MyList list2 = new MyList();
    provider.addListener(list1);
    provider.addListener(list2); 
    provider.notifyListeners();  // only list2 instance is notified

What is the best solution?

1. use another non-hashCode backing collection -- but WeakHashMap is so sweet for me, because automatically managing weak references for me

2. use non-generic listener, for example abstract class with simple equals() { return (this == object); } implementation -- but this is not so flexible

3. use some wrapper for listeners with simple equals() -- but this wrapper cannot be transparent to addListener(E) caller due to weak references

Another ideas?

Answer 1

The crux of the problem seems to be that your listener implementation is subclassing AbstractList, but not overriding equals() / hashCode(). I would strongly recommend against this type of inheritance (implementation inheritance) as it violates OO-principles (principle of polymorphic substitutability).

It would be far better to implement a custom listener class that possibly references an AbstractList if it requires one, and that also provides its own equals() and hashCode() implementations.

Answer 2

WeakHashMap is kind of broken. It uses weak keys, but it doesn't use identity hashing. Unless the equals() and hashCode() of your key type use "identity", you shouldn't use WeakHashMap. Instead you need something that's a combination of WeakHashMap and IdentityHashMap.

One possibility is to use MapMaker from Google Collections. It automatically uses identity hashes/equality for keys if the keys are weak or soft. eg:

ConcurrentMap<K, V> myMap = new MapMaker().weakKeys().makeMap();