From 0436d95ceae49f592f71531a164a716e61ea8066 Mon Sep 17 00:00:00 2001
From: "Ray J. Greenwell" <ray@threerings.net>
Date: Mon, 4 May 2015 17:48:00 -0700
Subject: [PATCH] Logging improvements for WeakObserverList.

- De-static checkedApply(). It should be overrideable and know
  other things about the ObserverList that it belongs to.
- Created an overrideable observerForLog().
- Have the DerefOp include the original Op's toString() in its own.
---
 .../com/samskivert/util/ObserverList.java     | 43 +++++++++++--------
 .../com/samskivert/util/WeakObserverList.java |  8 ++++
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/src/main/java/com/samskivert/util/ObserverList.java b/src/main/java/com/samskivert/util/ObserverList.java
index b15752e9..6fda999a 100644
--- a/src/main/java/com/samskivert/util/ObserverList.java
+++ b/src/main/java/com/samskivert/util/ObserverList.java
@@ -191,6 +191,30 @@ public abstract class ObserverList<T>
         return size() == 0;
     }
 
+    /**
+     * Applies the operation to the observer, catching and logging any exceptions thrown in the
+     * process.
+     */
+    protected boolean checkedApply (ObserverOp<T> obop, T obs)
+    {
+        try {
+            return obop.apply(obs);
+        } catch (Throwable thrown) {
+            log.warning("ObserverOp choked during notification",
+                    "op", obop, "obs", observerForLog(obs), thrown);
+            // if they booched it, definitely don't remove them
+            return true;
+        }
+    }
+
+    /**
+     * Get the object that should be logged as the observer.
+     */
+    protected Object observerForLog (T observer)
+    {
+        return observer;
+    }
+
     protected static class Impl<T> extends ObserverList<T> {
         protected Impl (Policy notifyPolicy) {
             _policy = notifyPolicy;
@@ -273,8 +297,8 @@ public abstract class ObserverList<T>
         protected boolean isDuplicate (T obs) {
             // make sure we're not violating the list constraints
             if (indexOf(obs) >= 0) {
-                log.warning("Observer attempted to observe list it's already observing!", "obs", obs,
-                            new Exception());
+                log.warning("Observer attempted to observe list it's already observing!",
+                        "obs", obs, new Exception());
                 return true;
             }
             return false;
@@ -284,19 +308,4 @@ public abstract class ObserverList<T>
         protected List<T> _list;
         protected boolean _checkDups = true;
     }
-
-    /**
-     * Applies the operation to the observer, catching and logging any exceptions thrown in the
-     * process.
-     */
-    protected static <T> boolean checkedApply (ObserverOp<T> obop, T obs)
-    {
-        try {
-            return obop.apply(obs);
-        } catch (Throwable thrown) {
-            log.warning("ObserverOp choked during notification", "op", obop, "obs", obs, thrown);
-            // if they booched it, definitely don't remove them
-            return true;
-        }
-    }
 }
diff --git a/src/main/java/com/samskivert/util/WeakObserverList.java b/src/main/java/com/samskivert/util/WeakObserverList.java
index ac3a1419..4f05aa46 100644
--- a/src/main/java/com/samskivert/util/WeakObserverList.java
+++ b/src/main/java/com/samskivert/util/WeakObserverList.java
@@ -108,6 +108,10 @@ public class WeakObserverList<T> extends ObserverList<T>
             return observer != null && _op.apply(observer);
         }
 
+        @Override public String toString () {
+            return "DerefOp:" + _op;
+        }
+
         /** The wrapped op. */
         protected ObserverOp<T> _op;
     }
@@ -128,6 +132,10 @@ public class WeakObserverList<T> extends ObserverList<T>
             }
             return -1;
         }
+
+        @Override protected Object observerForLog (WeakReference<T> ref) {
+            return ref.get();
+        }
     }
 
     /** A delegate list that contains weak reference wrapped elements. */