New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem using it with AndroidAnnotations #61

Closed
gdogaru opened this Issue Feb 15, 2013 · 16 comments

Comments

5 participants
@gdogaru

gdogaru commented Feb 15, 2013

I tried using dagger with AndroidAnnotations within activities.
The problem is that AndroidAnnotations creates a subclass of my activity.
When you look for annotated subscribers in com.squareup.otto.AnnotatedHandlerFinder#loadAnnotatedMethods you use listenerClass.getDeclaredMethods()
This does not return the methods from the super-class. So I can not receive messages if I create a subclass of the class that contains subscribe methods.
I got around this by registering an anonymous class with an annotated method.
Not sure if you want to support this, just mentioned it.
Thanks,
Gabi

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Feb 15, 2013

Collaborator

This will come with 2.0 when class hierarchy walking is restored. This was intentionally removed for the 1.x versions.

Collaborator

JakeWharton commented Feb 15, 2013

This will come with 2.0 when class hierarchy walking is restored. This was intentionally removed for the 1.x versions.

@tbsandee

This comment has been minimized.

Show comment
Hide comment
@tbsandee

tbsandee Feb 21, 2013

Contributor

I appreciate the decision, and it's easy enough to work around with enough care.

That said, gIven how easy it is to accidentally subscribe to events in a class and then subclass it, I wonder if either of these would be useful:

  • a warning could be logged (difficult, since you've kept dependencies minimal and there's no logging)
  • a "strict" flag for the Bus, where an IllegalArgumentException might be thrown
    if you try to subscribe to events using a non-final class.

In my code base, this is effectively what I've tried to do to avoid accidentally losing events.

Contributor

tbsandee commented Feb 21, 2013

I appreciate the decision, and it's easy enough to work around with enough care.

That said, gIven how easy it is to accidentally subscribe to events in a class and then subclass it, I wonder if either of these would be useful:

  • a warning could be logged (difficult, since you've kept dependencies minimal and there's no logging)
  • a "strict" flag for the Bus, where an IllegalArgumentException might be thrown
    if you try to subscribe to events using a non-final class.

In my code base, this is effectively what I've tried to do to avoid accidentally losing events.

@tbsandee

This comment has been minimized.

Show comment
Hide comment
@tbsandee

tbsandee Feb 21, 2013

Contributor

So, quick experiment leads me to the fact that anonymous inner classes are non-final (I'm sure there's a good reason). Oh well.

Contributor

tbsandee commented Feb 21, 2013

So, quick experiment leads me to the fact that anonymous inner classes are non-final (I'm sure there's a good reason). Oh well.

@imminent

This comment has been minimized.

Show comment
Hide comment
@imminent

imminent Feb 27, 2013

Contributor

@JakeWharton I seem to have an error with unregistering Subscribers using 2.0 snapshot. It's the IllegalArugmentException "Missing Subscriber...". Do you have to handle subscription differently in 2.0?

Contributor

imminent commented Feb 27, 2013

@JakeWharton I seem to have an error with unregistering Subscribers using 2.0 snapshot. It's the IllegalArugmentException "Missing Subscriber...". Do you have to handle subscription differently in 2.0?

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Feb 27, 2013

Collaborator

I wasn't even aware 2.0 compiled, to be honest.

Collaborator

JakeWharton commented Feb 27, 2013

I wasn't even aware 2.0 compiled, to be honest.

@imminent

This comment has been minimized.

Show comment
Hide comment
@imminent

imminent Feb 27, 2013

Contributor

The clean android code project uses it because the change is necessary for
integration with AndroidAnnotations, isn't it?
On Feb 27, 2013 1:39 PM, "Jake Wharton" notifications@github.com wrote:

I wasn't even aware 2.0 compiled, to be honest.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14201821
.

Contributor

imminent commented Feb 27, 2013

The clean android code project uses it because the change is necessary for
integration with AndroidAnnotations, isn't it?
On Feb 27, 2013 1:39 PM, "Jake Wharton" notifications@github.com wrote:

I wasn't even aware 2.0 compiled, to be honest.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14201821
.

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Feb 27, 2013

Collaborator

Yes, but it's a long way from actually being a shippable version of the library. It doesn't surprise me at all that there are problems with it. I wouldn't recommend using it until we move it to the master branch to dictate that it's relatively stable and pushing toward release.

Collaborator

JakeWharton commented Feb 27, 2013

Yes, but it's a long way from actually being a shippable version of the library. It doesn't surprise me at all that there are problems with it. I wouldn't recommend using it until we move it to the master branch to dictate that it's relatively stable and pushing toward release.

@imminent

This comment has been minimized.

Show comment
Hide comment
@imminent

imminent Feb 27, 2013

Contributor

That's what I was afraid of. I thought I would give it a try to find out
though. Would love to have these libraries working together. I'm all for
readable, maintainable code.
On Feb 27, 2013 2:07 PM, "Jake Wharton" notifications@github.com wrote:

Yes, but it's a long way from actually being a shippable version of the
library. It doesn't surprise me at all that there are problems with it. I
wouldn't recommend using it until we move it to the master branch to
dictate that it's relatively stable and pushing toward release.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14203331
.

Contributor

imminent commented Feb 27, 2013

That's what I was afraid of. I thought I would give it a try to find out
though. Would love to have these libraries working together. I'm all for
readable, maintainable code.
On Feb 27, 2013 2:07 PM, "Jake Wharton" notifications@github.com wrote:

Yes, but it's a long way from actually being a shippable version of the
library. It doesn't surprise me at all that there are problems with it. I
wouldn't recommend using it until we move it to the master branch to
dictate that it's relatively stable and pushing toward release.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14203331
.

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Feb 27, 2013

Collaborator

Well I'm starting actual 2.0 development today or tomorrow so it should move forward quickly. As it exists now it's more of a proof-of-concept.

Collaborator

JakeWharton commented Feb 27, 2013

Well I'm starting actual 2.0 development today or tomorrow so it should move forward quickly. As it exists now it's more of a proof-of-concept.

@imminent

This comment has been minimized.

Show comment
Hide comment
@imminent

imminent Feb 27, 2013

Contributor

Oh, awesome! Is there anything I can follow to see if I can contribute to
its progress. Annotation processing is a bit outside my usual domain. These
issues are for the whole repository, not just a branch, right?
On Feb 27, 2013 2:14 PM, "Jake Wharton" notifications@github.com wrote:

Well I'm starting actual 2.0 development today or tomorrow so it should
move forward quickly. As it exists now it's more of a proof-of-concept.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14203713
.

Contributor

imminent commented Feb 27, 2013

Oh, awesome! Is there anything I can follow to see if I can contribute to
its progress. Annotation processing is a bit outside my usual domain. These
issues are for the whole repository, not just a branch, right?
On Feb 27, 2013 2:14 PM, "Jake Wharton" notifications@github.com wrote:

Well I'm starting actual 2.0 development today or tomorrow so it should
move forward quickly. As it exists now it's more of a proof-of-concept.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14203713
.

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Feb 27, 2013

Collaborator

All issues relevant to its development will be organized on the 2.0 milestone of the project.

Collaborator

JakeWharton commented Feb 27, 2013

All issues relevant to its development will be organized on the 2.0 milestone of the project.

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Feb 27, 2013

Collaborator

I need to rebase it first, though, since the branch is about 6 months out of date.

Collaborator

JakeWharton commented Feb 27, 2013

I need to rebase it first, though, since the branch is about 6 months out of date.

@imminent

This comment has been minimized.

Show comment
Hide comment
@imminent

imminent Feb 27, 2013

Contributor

Got it.
On Feb 27, 2013 2:21 PM, "Jake Wharton" notifications@github.com wrote:

I need to rebase it first, though, since the branch is about 6 months out
of date.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14204069
.

Contributor

imminent commented Feb 27, 2013

Got it.
On Feb 27, 2013 2:21 PM, "Jake Wharton" notifications@github.com wrote:

I need to rebase it first, though, since the branch is about 6 months out
of date.


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-14204069
.

@WonderCsabo

This comment has been minimized.

Show comment
Hide comment
@WonderCsabo

WonderCsabo May 28, 2014

@JakeWharton what is the status of the 2.0 branch? Did you reject to continue development in that way?

@JakeWharton what is the status of the 2.0 branch? Did you reject to continue development in that way?

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton May 28, 2014

Collaborator

Nobody is working on it. We have no plans to release 2.0 at this time.
On May 28, 2014 12:24 PM, "Csaba Kozák" notifications@github.com wrote:

@JakeWharton https://github.com/JakeWharton what is the status of the
2.0 branch? Did you reject to continue development in that way?


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-44452825
.

Collaborator

JakeWharton commented May 28, 2014

Nobody is working on it. We have no plans to release 2.0 at this time.
On May 28, 2014 12:24 PM, "Csaba Kozák" notifications@github.com wrote:

@JakeWharton https://github.com/JakeWharton what is the status of the
2.0 branch? Did you reject to continue development in that way?


Reply to this email directly or view it on GitHubhttps://github.com/square/otto/issues/61#issuecomment-44452825
.

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Jan 8, 2015

Collaborator

#135 will restore hierarchy walking.

Collaborator

JakeWharton commented Jan 8, 2015

#135 will restore hierarchy walking.

@JakeWharton JakeWharton closed this Jan 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment