Changeset 11224 in josm for trunk/src

Timestamp:

2016-11-08T23:29:31+01:00 (8 years ago)

Author:

michael2402

Message:

Simplify ExpertToggleAction

Make ExpertToggleAction use the listener list to track weak listeners and add a method to set expert mode explicitly. Use it in OsmDataLayerTest to make tracing expert mode problems easier.

Add test case for ExpertToggleAction.

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/ExpertToggleAction.java (modified) (7 diffs)
• tools/ListenerList.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/actions/ExpertToggleAction.java

@@ -6 +6 @@
 import java.awt.Component;
 import java.awt.event.ActionEvent;
-import java.lang.ref.WeakReference;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.preferences.BooleanProperty;
+import org.openstreetmap.josm.tools.ListenerList;
 
 /**
@@ -19 +17 @@
 public class ExpertToggleAction extends ToggleAction {
 
+    /**
+     * This listener is notified whenever the expert mode setting changed.
+     */
     @FunctionalInterface
     public interface ExpertModeChangeListener {
+        /**
+         * The expert mode changed.
+         * @param isExpert <code>true</code> if expert mode was enabled, false otherwise.
+         */
         void expertChanged(boolean isExpert);
     }
 
-    private static final List<WeakReference<ExpertModeChangeListener>> listeners = new ArrayList<>();
-    private static final List<WeakReference<Component>> visibilityToggleListeners = new ArrayList<>();
+    // TODO: Switch to checked list. We can do this as soon as we do not see any more warnings.
+    private static final ListenerList<ExpertModeChangeListener> listeners = ListenerList.createUnchecked();
+    private static final ListenerList<Component> visibilityToggleListeners = ListenerList.createUnchecked();
+
+    private static final BooleanProperty PREF_EXPERT = new BooleanProperty("expert", false);
 
     private static final ExpertToggleAction INSTANCE = new ExpertToggleAction();
 
     private static synchronized void fireExpertModeChanged(boolean isExpert) {
-        Iterator<WeakReference<ExpertModeChangeListener>> it1 = listeners.iterator();
-        while (it1.hasNext()) {
-            WeakReference<ExpertModeChangeListener> wr = it1.next();
-            ExpertModeChangeListener listener = wr.get();
-            if (listener == null) {
-                it1.remove();
-                continue;
-            }
-            listener.expertChanged(isExpert);
-        }
-        Iterator<WeakReference<Component>> it2 = visibilityToggleListeners.iterator();
-        while (it2.hasNext()) {
-            WeakReference<Component> wr = it2.next();
-            Component c = wr.get();
-            if (c == null) {
-                it2.remove();
-                continue;
-            }
-            c.setVisible(isExpert);
-        }
+        listeners.fireEvent(listener -> listener.expertChanged(isExpert));
+        visibilityToggleListeners.fireEvent(c -> c.setVisible(isExpert));
     }
 
@@ -63 +53 @@
     public static synchronized void addExpertModeChangeListener(ExpertModeChangeListener listener, boolean fireWhenAdding) {
         if (listener == null) return;
-        for (WeakReference<ExpertModeChangeListener> wr : listeners) {
-            // already registered ? => abort
-            if (wr.get() == listener) return;
-        }
-        listeners.add(new WeakReference<>(listener));
+        listeners.addWeakListener(listener);
         if (fireWhenAdding) {
             listener.expertChanged(isExpert());
@@ -80 +66 @@
     public static synchronized void removeExpertModeChangeListener(ExpertModeChangeListener listener) {
         if (listener == null) return;
-        Iterator<WeakReference<ExpertModeChangeListener>> it = listeners.iterator();
-        while (it.hasNext()) {
-            WeakReference<ExpertModeChangeListener> wr = it.next();
-            // remove the listener - and any other listener which god garbage
-            // collected in the meantime
-            if (wr.get() == null || wr.get() == listener) {
-                it.remove();
-            }
-        }
+        listeners.removeListener(listener);
     }
 
+    /**
+     * Marks a component to be only visible when expert mode is enabled. The visibility of the component is changed automatically.
+     * @param c The component.
+     */
     public static synchronized void addVisibilitySwitcher(Component c) {
         if (c == null) return;
-        for (WeakReference<Component> wr : visibilityToggleListeners) {
-            // already registered ? => abort
-            if (wr.get() == c) return;
-        }
-        visibilityToggleListeners.add(new WeakReference<>(c));
+        visibilityToggleListeners.addWeakListener(c);
         c.setVisible(isExpert());
     }
 
+    /**
+     * Stops tracking visibility changes for the given component.
+     * @param c The component.
+     * @see #addVisibilitySwitcher(Component)
+     */
     public static synchronized void removeVisibilitySwitcher(Component c) {
         if (c == null) return;
-        Iterator<WeakReference<Component>> it = visibilityToggleListeners.iterator();
-        while (it.hasNext()) {
-            WeakReference<Component> wr = it.next();
-            // remove the listener - and any other listener which god garbage
-            // collected in the meantime
-            if (wr.get() == null || wr.get() == c) {
-                it.remove();
-            }
-        }
+        visibilityToggleListeners.removeListener(c);
     }
 
@@ -128 +103 @@
             Main.toolbar.register(this);
         }
-        setSelected(Main.pref.getBoolean("expert", false));
+        setSelected(PREF_EXPERT.get());
         notifySelectedState();
     }
@@ -135 +110 @@
     protected final void notifySelectedState() {
         super.notifySelectedState();
+        PREF_EXPERT.put(isSelected());
         fireExpertModeChanged(isSelected());
+    }
+
+    /**
+     * Forces the expert mode state to the given state.
+     * @param isExpert if expert mode should be used.
+     * @since 11224
+     */
+    public void setExpert(boolean isExpert) {
+        if (isSelected() != isExpert) {
+            setSelected(isExpert);
+            notifySelectedState();
+        }
     }
 
@@ -141 +129 @@
     public void actionPerformed(ActionEvent e) {
         toggleSelectedState(e);
-        Main.pref.put("expert", isSelected());
         notifySelectedState();
     }

trunk/src/org/openstreetmap/josm/tools/ListenerList.java

@@ -80 +80 @@
      */
     public synchronized void addWeakListener(T listener) {
-        ensureNotInList(listener);
-        // clean the weak listeners, just to be sure...
-        while (weakListeners.remove(new WeakListener<T>(null))) {
-            // continue
-        }
-        weakListeners.add(new WeakListener<>(listener));
+        if (ensureNotInList(listener)) {
+            // clean the weak listeners, just to be sure...
+            while (weakListeners.remove(new WeakListener<T>(null))) {
+                // continue
+            }
+            weakListeners.add(new WeakListener<>(listener));
+        }
     }
 
@@ -93 +94 @@
      */
     public synchronized void addListener(T listener) {
-        ensureNotInList(listener);
-        listeners.add(listener);
-    }
-
-    private void ensureNotInList(T listener) {
+        if (ensureNotInList(listener)) {
+            listeners.add(listener);
+        }
+    }
+
+    private boolean ensureNotInList(T listener) {
         CheckParameterUtil.ensureParameterNotNull(listener, "listener");
         if (containsListener(listener)) {
             failAdd(listener);
+            return false;
+        } else {
+            return true;
         }
     }
@@ -217 +222 @@
     }
 
+    private static class UncheckedListenerList<T> extends ListenerList<T> {
+        @Override
+        protected void failAdd(T listener) {
+            Logging.warn("Listener was alreaady added: {0}", listener);
+            // ignore
+        }
+
+        @Override
+        protected void failRemove(T listener) {
+            Logging.warn("Listener was removed twice or not added: {0}", listener);
+            // ignore
+        }
+    }
+
     /**
      * Create a new listener list
@@ -229 +248 @@
         }
     }
+
+    /**
+     * Creates a new listener list that does not fail if listeners are added ore removed twice.
+     * <p>
+     * Use of this list is discouraged. You should always use {@link #create()} in new implementations and check your listeners.
+     * @param <T> The listener type
+     * @return A new list.
+     * @since 11224
+     */
+    public static <T> ListenerList<T> createUnchecked() {
+        return new UncheckedListenerList<>();
+    }
 }