Edge case in EventBus#unregister

[GoogleCodeExporter]

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.

Original issue reported on code.google.com by stephan...@gmail.com on 6 Jan 2014 at 10:22

[GoogleCodeExporter]

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.

Original comment by cgdecker@google.com on 6 Jan 2014 at 6:16

[GoogleCodeExporter]

> 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.

Original comment by Maaarti...@gmail.com on 8 Jan 2014 at 7:50

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

• Changed state: Migrated

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07

• Added labels: Restrict-AddIssueComment-Commit