added support for inheritance

[SierraGolf]

This is a solution to the limitation that otto only processes methods of the immediate type, disregarding any annotated super methods. This topic has been discussed here.

This pull request adds an annotation to the public API of otto which allows the user to explicitly declare classes for which otto should check super classes for annotations (producers/subscribers).

Credits for the original implementation of this pull request go to @deadfalkon.

[csobrinho]

+1

[slapsys]

👍 Would love to see this merged.

[falkorichter]

Looks nice, as someone related to this pull-request I approve it 👍

[Disruption]

👍 Seems very nice!

[tobiasheine]

+1

[paddyzab]

+1

[SierraGolf]

@JakeWharton can you give some feedback on this PR?

[JakeWharton]

I don't like this approach, personally. I'd rather just walk up the class hierarchy until you hit java.* or android.* and then stop.

[Disruption]

But that approach only gives you the option of "1 class up, or all the way up", while this approach lets you customize where you stop. Why is that worse?

Anyways, the annotation could be parametrized, or a new annotation could be added to support both behaviours (one class up, all the way up), I don't think they are mutually exclusive.

[rjrjr]

-1. This is clunky. Requiring subclasses to know which superclasses, if any, need to subscribe is almost no improvement over the current situation.

[JakeWharton]

I meant no annotation, always walk up until you hit android or java.

[Disruption]

@JakeWharton But isn't that not done for the sake of performance, as you commented on #26 (comment) ? That's why the annotation seems like a nice idea, so the people not needing to traverse the whole hierarchy can just keep the one level code inspection/method traversing, and the ones wanting to go higher can manually activate it (Either fully hierarchy as you suggest, or one level, whatever).

Cheers!

[SierraGolf]

@JakeWharton / @rjrjr I agree that the approach to go to a set of specified packages, where the default would be java.* and android.* is a better implementation. It will actually make Otto more of a framework which follows the "principle of least astonishment" (it will simply work, no further annotation needed).

Maybe in a first draft the packages can be hardcoded and be made configurable as people start to see the need for that?

If you guys agree, I will provide the above mentioned alternative implementation to this PR.

[thalespf]

+1

[tlahoda]

Anyway you want to do it (annotation or walk the path up to Object for all I care), can we please get this functionality in? Otto is mostly useless to me without it and I love the concept and how it has been done. The apps I am writing will benefit from Otto and this if we can get it in.

[SierraGolf]

@JakeWharton / @rjrjr any feedback on my last comment, it has been a couple of month.

[imminent]

So, a nice advantage to the current approach to dealing with this superclass subscription is that it is dependable. The current approach is to register a class field instead of the superclass itself:

class SomeSuperClass {
  private final SomeEventListener busListener;

  SomeSuperClass(Bus bus) {
    busListener = new SomeEventListener();
    bus.register(busListener);
  }

  public static class SomeEventListener {
    @Subscribe public void onEvent(SomeEvent event) {
      // do something
    }
  }
}

This is dependable because even if some subclass SomeSubClass extends SomeSuperClass comes along later and subscribes to SomeEvent, the SomeEventListener will still get called (as well). This statement can't be made for the other approach that traverses the class hierarchy, because there's but a single object. See how in this case, no subclass of SomeSuperClass can be written in a way that prevents "// do something" from happening when SomeEvent happens.

[Disruption]

@imminent Following the same reasoning, that also implies that you can not override the "onEvent -> doSomething" in order to call super and then do your things, but to listen to the event separately in two ways, which in turn means you don't know in what order it will be called, which is bad for a self-contained scenario. (That is, in the case of extensions, you usually want to call the super code first, and then your own if any). With the approach of multiple one-level listeners subscribed for specific behaviour, you have a 'random' execution order.

Of course you can solve that in some elegant ways, like adding an "afterEvent" abstract method, and have it implemented, and so on, but sometimes it's not "so trivial".

I think that having the option of choosing if you want to traverse the hierarchy is good, as it's not forced to the user in any way. A user wanting that behaviour have to explicitly annotate the class to get it, if not, he will get the current behaviour, which seems fair enough.

[imminent]

@Disruption actually there is a way to override an onEvent method if that's desired, and it's very simple.

public class SomeSuperClass {

  public void onEvent(SomeEvent event) {
      // do something
  }
}

public class SomeSubClass extends SomeSuperClass {

  SomeSubClass(Bus bus) {
    bus.register(this);
  }

  @Subscribe @Override
  public void onEvent(SomeEvent event) {
    super.onEvent(event);
    // do some more stuff
  }
}

But this @Subscribe methods should prefer to be constructed in a self-contained manner that doesn't require extension, and doesn't care about the order they are triggered. Actually, you can you RxJava for Reactive programming if you a similar paradigm that does give you control over order of operation (it's similar because of the Observable model).

[JakeWharton]

We're not going an opt-in route. #135.