Committed by
Gerrit Code Review
Simplifying removeListener to merely log when remove indicates no such listener.
Change-Id: I6d47fd70938b5e049e523f94d3e7242661fb154d
Showing
3 changed files
with
3 additions
and
57 deletions
| ... | @@ -20,7 +20,6 @@ import org.slf4j.Logger; | ... | @@ -20,7 +20,6 @@ import org.slf4j.Logger; |
| 20 | import java.util.Set; | 20 | import java.util.Set; |
| 21 | import java.util.concurrent.CopyOnWriteArraySet; | 21 | import java.util.concurrent.CopyOnWriteArraySet; |
| 22 | 22 | ||
| 23 | -import static com.google.common.base.Preconditions.checkArgument; | ||
| 24 | import static com.google.common.base.Preconditions.checkNotNull; | 23 | import static com.google.common.base.Preconditions.checkNotNull; |
| 25 | import static org.slf4j.LoggerFactory.getLogger; | 24 | import static org.slf4j.LoggerFactory.getLogger; |
| 26 | 25 | ||
| ... | @@ -33,8 +32,6 @@ public class ListenerRegistry<E extends Event, L extends EventListener<E>> | ... | @@ -33,8 +32,6 @@ public class ListenerRegistry<E extends Event, L extends EventListener<E>> |
| 33 | 32 | ||
| 34 | private final Logger log = getLogger(getClass()); | 33 | private final Logger log = getLogger(getClass()); |
| 35 | 34 | ||
| 36 | - private volatile boolean shutdown = false; | ||
| 37 | - | ||
| 38 | private long lastStart; | 35 | private long lastStart; |
| 39 | private L lastListener; | 36 | private L lastListener; |
| 40 | 37 | ||
| ... | @@ -61,8 +58,8 @@ public class ListenerRegistry<E extends Event, L extends EventListener<E>> | ... | @@ -61,8 +58,8 @@ public class ListenerRegistry<E extends Event, L extends EventListener<E>> |
| 61 | */ | 58 | */ |
| 62 | public void removeListener(L listener) { | 59 | public void removeListener(L listener) { |
| 63 | checkNotNull(listener, "Listener cannot be null"); | 60 | checkNotNull(listener, "Listener cannot be null"); |
| 64 | - if (checkForNonRegistrant()) { | 61 | + if (!listeners.remove(listener)) { |
| 65 | - checkArgument(listeners.remove(listener), "Listener not registered"); | 62 | + log.warn("Listener {} not registered", listener); |
| 66 | } | 63 | } |
| 67 | } | 64 | } |
| 68 | 65 | ||
| ... | @@ -91,41 +88,13 @@ public class ListenerRegistry<E extends Event, L extends EventListener<E>> | ... | @@ -91,41 +88,13 @@ public class ListenerRegistry<E extends Event, L extends EventListener<E>> |
| 91 | } | 88 | } |
| 92 | 89 | ||
| 93 | /** | 90 | /** |
| 94 | - * Predicate indicating whether we should throw an exception if the | ||
| 95 | - * argument to {@link #removeListener} is not in the current set of | ||
| 96 | - * listeners. | ||
| 97 | - * <p> | ||
| 98 | - * This default implementation returns <code>true</code>. | ||
| 99 | - * | ||
| 100 | - * @return true if non-listed listeners should cause exception on remove | ||
| 101 | - */ | ||
| 102 | - protected boolean checkForNonRegistrant() { | ||
| 103 | - return true; | ||
| 104 | - } | ||
| 105 | - | ||
| 106 | - /** | ||
| 107 | * Reports a problem encountered while processing an event. | 91 | * Reports a problem encountered while processing an event. |
| 108 | * | 92 | * |
| 109 | * @param event event being processed | 93 | * @param event event being processed |
| 110 | * @param error error encountered while processing | 94 | * @param error error encountered while processing |
| 111 | */ | 95 | */ |
| 112 | protected void reportProblem(E event, Throwable error) { | 96 | protected void reportProblem(E event, Throwable error) { |
| 113 | - if (!shutdown) { | 97 | + log.warn("Exception encountered while processing event " + event, error); |
| 114 | - log.warn("Exception encountered while processing event " + event, error); | ||
| 115 | - } | ||
| 116 | } | 98 | } |
| 117 | 99 | ||
| 118 | - /** | ||
| 119 | - * Prepares the registry for normal operation. | ||
| 120 | - */ | ||
| 121 | - public void activate() { | ||
| 122 | - shutdown = false; | ||
| 123 | - } | ||
| 124 | - | ||
| 125 | - /** | ||
| 126 | - * Prepares the registry for shutdown. | ||
| 127 | - */ | ||
| 128 | - public void deactivate() { | ||
| 129 | - shutdown = true; | ||
| 130 | - } | ||
| 131 | } | 100 | } | ... | ... |
| ... | @@ -26,13 +26,6 @@ import static org.junit.Assert.assertTrue; | ... | @@ -26,13 +26,6 @@ import static org.junit.Assert.assertTrue; |
| 26 | */ | 26 | */ |
| 27 | public class ListenerRegistryTest { | 27 | public class ListenerRegistryTest { |
| 28 | 28 | ||
| 29 | - private static class RelaxedRegistry extends TestListenerRegistry { | ||
| 30 | - @Override | ||
| 31 | - protected boolean checkForNonRegistrant() { | ||
| 32 | - return false; | ||
| 33 | - } | ||
| 34 | - } | ||
| 35 | - | ||
| 36 | private static final TestEvent FOO_EVENT = | 29 | private static final TestEvent FOO_EVENT = |
| 37 | new TestEvent(TestEvent.Type.FOO, "foo"); | 30 | new TestEvent(TestEvent.Type.FOO, "foo"); |
| 38 | private static final TestEvent BAR_EVENT = | 31 | private static final TestEvent BAR_EVENT = |
| ... | @@ -78,16 +71,4 @@ public class ListenerRegistryTest { | ... | @@ -78,16 +71,4 @@ public class ListenerRegistryTest { |
| 78 | assertTrue("BAR not processed", secondListener.events.contains(BAR_EVENT)); | 71 | assertTrue("BAR not processed", secondListener.events.contains(BAR_EVENT)); |
| 79 | } | 72 | } |
| 80 | 73 | ||
| 81 | - @Test(expected = IllegalArgumentException.class) | ||
| 82 | - public void removeNonListenerCausesException() { | ||
| 83 | - manager.removeListener(listener); | ||
| 84 | - } | ||
| 85 | - | ||
| 86 | - @Test | ||
| 87 | - public void removeNonListenerIgnored() { | ||
| 88 | - manager = new RelaxedRegistry(); | ||
| 89 | - manager.removeListener(listener); | ||
| 90 | - assertTrue("what?", manager.listeners.isEmpty()); | ||
| 91 | - } | ||
| 92 | - | ||
| 93 | } | 74 | } | ... | ... |
-
Please register or login to post a comment