Description
Hey, this looks like a great library, and I have been exploring using it. I know you guys have quite done quite a bit of research regarding Anotations vs Method Naming, but overall I really feel like Annotations are a much cleaner and more flexible solution.
I added annotations to my local branch. You can create a new EventBus that uses annotations like this new EventBus(DeclarationType.ANNOTATIONS). Then you can subscribe to receive events on any method like this:
@Subscribe
public void onMyEvent(CustomEvent e)
This uses the caller chooses thread method. We can also change this by adding an argument.
@Subscribe(threadMode = ThreadMode.Background)
public void onMyEvent(CustomEvent e)
This uses the background thread and is definitely a more explicit syntax.
Here is a link to my repo where I implemented the changes.
https://github.com/rjbrock/EventBus
I would still need to add unit tests before submitting a pull request so I just wanted to see what your thoughts were first.
Rick
Metadata
Metadata
Assignees
Labels
Projects
Relationships
Development
Participants
Activity
greenrobot commentedon Nov 12, 2014
Awesome! Annotations were planned for EventBus version 3. It makes sense today, because Android 4 is adopted widely these days. With Android 4, the performance difference should not really matter anymore. You can verify with the performance app project. Looking forward to check your changes, however it might get December - so no hurry. We just did a release anyway. Thanks!
rjbrock commentedon Nov 12, 2014
Sounds good! Here is a link to the actual changeset.
rjbrock@e42992a
If things look ok (so far) I can flesh this out with more comments and unit test coverage as well as docs.
greenrobot commentedon Nov 12, 2014
I see it's possible to do either method names searching and Annotations? That's nice for transition. I'd suggest to move that switch to the builder. The forthcoming release (e.g. 3.1) can prune the method name option afterwards.
Let's think a bit about consequences of adopting annotations:
What do you think?
PS.: Please ensure you are working on the latest version, there were some changes in this area...
[-]Question about Annotations[/-][+]Annotation Support[/+]greenrobot commentedon Nov 12, 2014
Giving it some more thought, maybe we should just drop method name searching completely? It will get messy supporting both. Forcing people to add an annotation to their onEvent method should not be a big problem, I think.
rjbrock commentedon Nov 12, 2014
I am totally on board with dropping method name searching completely. My goal with the original changeset that I linked you to was to be as minimally intrusive as possible.
I will update my changes to remove method name searching altogether then.
Do you want me working off of my repo for now? I will create an annotations branch.
greenrobot commentedon Nov 12, 2014
Sounds good. Next weeks will be busy for me, but I'm really looking forward to have a look.
MariusVolkhart commentedon Dec 29, 2014
How is this going to work with inheritance? When using Otto for example, you can't put an annotation on a superclass and have the subclass get the message because Otto doesn't traverse the class hierarchy for performance reasons.
rjbrock commentedon Dec 29, 2014
@MariusVolkhart This doesn't change any behavior. This only changes the way that reflection is used to find the methods that are subscribing.
19 remaining items