Eventbus: Canceling Events only work partly

[timroes]

If you call EventBus.cancelEventDelivery() the 'event' won't be canceled completely. Only subscribers, who have subscribed to exactly that event class (and not a parent one of that) won't get notified. If you have e.g. an onEvent(Object ev) listener and canceling sending of MyCustomEvent in an onEvent(MyCustomEvent) the onEvent(Object) method will still receive it. This is (imho) pretty unexpected behavior.

The error lies here: https://github.com/greenrobot/EventBus/blob/master/EventBus/src/de/greenrobot/event/EventBus.java#L482

Your break only break out the inner loop (subscribers for this class) and not the outer loop (all parent classes of the current event). You should also break out of the outer for loop.

Didn't know if you wish a labeled loop or putting the abort to the for condition, that's why I didn't send a pull request (and because it's anyway such a small change).

Unfortunately I didn't find a workaround till this is fixed, to really cancel an event. It seems like you have to keep an own list of canceled events if you use event hierarchy.

I hope the fix can be published soon :-) Thanks guys for the great lib.

[timroes]

Eh okay, labeled loops seem not to work in dalvik, but a plain return would also help, since subscriptionFound will always be true if this line is reached.

[Panthaaaa]

Current location of the erroneous line is: https://github.com/greenrobot/EventBus/blob/master/EventBus/src/org/greenrobot/eventbus/EventBus.java#L404

You could possibly just throw an IsCanceledException and catch it in the postSingleEvent-Loop.

[william-ferguson-au]

The simple solution is to not use event inheritance.

[Panthaaaa]

I don't think to solution to a buggy feature is to just stop using it. IMHO it's really helpful to use event inheritance in larger applications (i.e. complex event hierachies) to manage the complexity.

I will add a pull requests with a fix, so this could be integrated into one of the next versions.

[william-ferguson-au]

IMHO poor use of inheritance is to blame for a good chunk of the failings of modern OO programs. IMHO event inheritance sits right in those cross hairs. You are creating a brittle application.

But hell it's your app.

[Panthaaaa]

Thank You for your opinion, but I'm not sure I understand you correctly.
Could you elaborate why you think event inheritance is bad? Or how you think events should be structured?

[william-ferguson-au]

It sounds like you are either
a) attempting to have an Event represent two or more discrete events
b) have created a Swiss army knife consumers that is trying to respond to an event multiple times in distinct ways.

The first is solved by separating your Event into the underlying Events it actually represents and post them each in turn.

The second is solved by combining the functionality in the multiple subscriber methods in your consumer is a single subscriber for the concrete Event class.

I wouldn't be (and we don't) use Event inheritance at all. Instead all subscriptions are for a concrete Event class. It makes it very easy to determine who the producers and who the consumers are when you want to start aggressively refactoring.

You also gain certainty about execution order, because you are never wondering which subscriber is called before another as there is only one subscriber (in a consumer) for an Event.

There's more, but those are the highlights.

[Panthaaaa]

We do neither of those two points, but something a little different:

When we get an Exception during the execution of a request we typically want to show a toast to the user. Therefore we send e.g. a PriceSaveException extends BusinessException as an event. The DefaultBusinessExceptionHandler subscribes on BusinessException and shows a Toast.

If we want to extend the behaviour by e.g. showing an additional Dialog, we have a method subscribing to PriceSaveException which then shows the Dialog and then the DefaultBusinessExceptionHandler shows the Toast .
The execution order is clear, as the object hierarchy is used, so PriceSaveException-method will be called before BusinessException-method.

If we want to replace the behaviour by e.g. only showing a Dialog, we have a method subscribing to PriceSaveException which then shows the Dialog and consumes the event (== cancelEventDelivery(event)).

This is when the bug occurs and breaks the Event & Subscriber inheritance feature.

[william-ferguson-au]

Sounds fragile. Think about what happens when a developer that is not offay with the way you have coupled the event inheritance hierarchy to business logic changes the inheritance path.