Suppose I am listening for network changes with the following listener:interface NetListener {    void onNetworkAvailable(boolean isWifi);    void onNetworkUnavailable();}I want to have an object, accessible from anywhere within the app, holding the registered NetListeners, that can trigger the appropriate methods when necessary. The solution I came up with is this singleton: class NetListenerManager {    private static List<NetListener> _listeners = new CopyOnWriteArrayList<NetListener>();    static void addListener(final NetListener listener) {        if (null != listener) { _listeners.add(listener); }    }    static void removeListener(final NetListener listener) {        if (null != listener) { _listeners.remove(listener); }    }    /**     * Called when a network connection is available.     */    static void notifyOnNetworkAvailable(boolean isWifi) {        for (NetListener listener : _listeners) { listener.onNetworkAvailable(isWifi); }    }    /**     * Called when a network connection becomes unavailable.     */    static void notifyOnNetworkUnavailable() {        for (NetListener listener : _listeners) { listener.onNetworkUnavailable(); }    }    private NetListenerManager() { /* Instantiation disabled. */ }}So whenever e.g. a network change broadcast is received, I call NetListenerManager.notifyOnNetworkAvailable or NetListenerManager.notifyOnNetworkUnavailable, depending on the network state. Im wondering if this is a good solution or if there are better ways to implement this. Thanks!UPDATE no.2This is the modified listener: interface NetListener {    static enum NetworkType {NO_NETWORK, WIFI, MOBILE}    void onNetworkChange(NetworkType type);}And the manager:public final class NetListenerManager {    private static final List<NetListener> NET_LISTENERS = new CopyOnWriteArrayList<NetListener>();    private static final BlockingQueue<NetListener.NetworkType> QUEUE = new LinkedBlockingQueue<NetListener.NetworkType>();    private static final Thread QUEUE_PROCESSOR = new Thread(new QueueProcessor());    static { QUEUE_PROCESSOR.start(); }    public static void addListener(final NetListener listener) {        if (null != listener) { NET_LISTENERS.add(listener); }    }    public static void removeListener(final NetListener listener) {        if (null != listener) { NET_LISTENERS.remove(listener); }    }    /**     * Called when there is a network change.     */    public static void notifyOnNetworkChange(final NetListener.NetworkType type) {        QUEUE.add(type);    }    private NetListenerManager() { /* Instantiation disabled. */ }    /**     * This Runnable propagates network change events to the registered NetListener listeners in a queue.     */    private static final class QueueProcessor implements Runnable {        @Override        public void run() {            while (true) {                try {                    final NetListener.NetworkType type = QUEUE.take();                    for (NetListener listener : NET_LISTENERS) { listener.onNetworkChange(type); }                } catch (InterruptedException ie) { /* Do nothing */ }            }        }    }}
Singleton holding and notifying registered listeners
java;performance;android
In general this is a good solution, but there are a few points you should consider:General style: Make the class final. There is no reason for anyone to subclass it, and the private Constructor shows that intent, but making the class itself final is a good thing.Similarly, make the _listeners list final as well.The next item is whether you can guarantee that the static methods on your class will only ever be called from a single thread. If you can guarantee this then the code will be good. If not, then it is possible for there to be 'transient' situations where the network 'blips', sending a 'network down' followed by a 'network up' notification. If they come from different threads then it is possible the the NetListener instances may get the notifications out of order.For a thread-safe system I would create an enum with three states:enum (NET_DOWN, NET_UP_WIFI, NET_UP_REGULAR);and then feed those states on to a Concurrent*Queue on the notify side. Then, have a seperate thread that reads from that queue (guaranteeing the order) and feeds the events to the listeners.This also removes potentially long-running method calls from the service that provides you the notification events.... (i.e. this may make the OS more responsive to other applications)EDITAfter your edit you have introduced the concept of a separate thread for managing the notification process.The only comment I have for the modification is that the java.util.concurrent typically has 'the right tool for the job' somewhere. In this case, you chose the wrong tool. It is pretty complicated sometimes, but the right tool would probably be an implementation of a BlockingQueue. In this case I would choose LinkedBlockingQueue. Your methods can then lose the synchronization blocks and simply become:public static void notifyOnNetworkChange(final NetListener.NetworkType type) {    QUEUE.put(type);}and your read-side becomes (note you had a bug before because if you had multiple listeners you would poll different values for each of them):while(...) {    NetworkType state = QUEUE.take();    for (NetListener listener : NET_LISTENERS) { listener.onNetworkChange(state);}The LinkedBlockingQueue deals with all the synchronization issues for you.