Edge case in EventBus#unregister

[gissuebot]

Original issue created by stephan202 on 2014-01-06 at 10:22 AM

---

Scenario: in some unit tests, I'd like to test whether some object was (un)registered with an EventBus, similar to Issue 784. I use a construction akin to the following (I am aware of this method's side effect, that's fine for my purposes):

private void verifyRegistration(final EventBus eventBus, final Object object, final boolean expectedRegistered) {
    try {
        eventBus.unregister(object);
        assertTrue(expectedRegistered);
    } catch (final IllegalArgumentException e) {
        assertFalse(expectedRegistered);
    }   
}   

Now, it turns out that this does not work in case the object of interest does not define any @Subscribe methods. The documentation for EventBus#unregister states that an IllegalArgumentException is thrown "if the object was not previously registered", but in this case no Exception is thrown. The following two unit tests illustrate the issue more clearly (only the first one passes):

@Test(expected = IllegalArgumentException.class)
public void testUnregisterWithSubscriber() {
    new EventBus().unregister(new Object() {
        @Subscribe
        public void handleEvent(final Object event) { } 
    }); 
}   

@Test(expected = IllegalArgumentException.class)
public void testUnregisterWithNonSubscriber() {
    new EventBus().unregister(new Object());
}   

NB: I was torn between labeling this issue Type-Defect and Type-ApiDocs. Went with the former as I think a code fix is to be preferred over a documentation fix.

[gissuebot]

Original comment posted by cgdecker@google.com on 2014-01-06 at 06:16 PM

---

Hmm... the problem with doing this is that we don't currently store anything at all when an object has no @Subscribe methods, because we only need to store event type -> subscriber mappings. Having to add a whole new Set<Object> for everything that's been passed to register just to make this work seems undesirable to me, though it wouldn't be the end of the world. It does seem slightly preferable for unregister to be throwing an exception if the object was never passed to register regardless of whether or not it has an @Subscribe method though.

[gissuebot]

Original comment posted by Maaartinus on 2014-01-08 at 07:50 PM

---

Having to add a whole new Set<Object> for everything that's been passed to register

It would be enough to store the non-subscribers. However, they may constitute the majority as it's common to register all newly created objects without any thoughts.

There's an asymmetry in the behavior: While unregister seems to try to protect people from accidentally passing a wrong object, register accepts everything and even allows registering twice (which is a no-op). Assuming there's a good reason for un-registering throwing (which I personally doubt), the same reason should apply for re-registering.

[cgdecker]

I'm closing this issue because we are no longer going to be making changes to EventBus other than important bug fixes. The reasons we're discouraging it and some alternatives are listed in the EventBus javadoc.