-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Provide constructor injection and setter injection (?) #204
Comments
I vote for it. Great idea.
|
Note: I'm note sure what's best, the setter annotation could also be directly on the setter instead of being on the argument : @ViewById
void setMyView(View myView) {
this.myView = myView;
} The latter is more common, but the former gives more flexibility. |
Any progress on this isue? |
Hi @giejay, This feature is not scheduled for 3.0, so I guess not any work has been started on this. However, you're really welcome to contribute :) |
i had some thoughts about this issue. setter
constructor
I think i'd give the setter injection via annotation on method a try with code like @pyricau suggested. @ViewById
void setMyView(View myView) {
this.myView = myView;
} but i would not call it "setter injection" but rather "method injection" allowing any name for the method and use the parameter name as default for annotation values like @Extra("MY_EXTRA_KEY")
void reactOnExtra(String extraValue) {
// do stuff with extraValue
} that currently has to be @Extra("MY_EXTRA_KEY")
String extraValue;
@AfterExtras
void reactOnExtra() {
// do stuff with extraValue
} while the code saved is minimal in this example the upper way would not pollute the class scope with a variable only used in one method. but as mentioned above, this could also cause unexpected issues (mostly NullPointerExceptions) when not used carefully. one should only use this as setter or only work with the value that got injected or are otherwise ensured to be set. |
8 months without feedback. I just started to play with method injection for |
Sorry, @dodgex i did not saw this issue.
Why? We already have multiple annotations working just fine, and i just added two more. |
@WonderCsabo actualy i'm not sure why i wrote that. iirc there was the discussion if we want to have annotations on a parameter and we decided to have them after this comment. btw: if you want to see the current implementation you can see it here: https://github.com/dodgex/androidannotations/tree/204_method_injection |
@dodgex it is a good idea. :) I see no problems with it. Actually i think it is much more natural then the method annotation. This is used by every DI system. |
@WonderCsabo uhm, i'm not sure if i understand your comment. what is more natural? annotation on param? |
Yeah, the param, of course. Becase the value actually will be injected to the parameter. Just like with |
okay. shall i create something like |
What is the problem with this? void setBeans(@Bean(MyBeanImpl.class) MyBean bean, @Bean(MyBeanOtherImpl.class) MyBean bean2) {
this.bean = bean;
this. bean2 = bean2;
} 😕 |
FYI: maybe you should have a look at this branch. #883 (comment). |
@WonderCsabo the problem i currently see is that i have no idea how to implement the process. that way we would get two calls to process. one for each parameter. but the result should be only one call to |
Just create a map with setter methods, and create the method lazily in it. This is done by lot of handlers actually. |
hu? can you tell me one handler that does that, that i can see how it is done? |
You can check out how |
they use the same call/return value. but in this case it is the other way. we would need to have 1 call with multiple parameters. but yeah, that should be doable in a similar way... |
Yeah, but the idea is the same about lazy generating the method call itself. BTW, you can check out google/dagger and transfuse projects to get some ideas. |
hmm for some reason the annotations on param only get validated, but not processed. do you have an idea why? |
I guess the problem that we already registered the same annotation for fields. For now create a new annotation ( |
i debugged it: the problem is that the ModelProcessor is currently not able to handle method parameters. |
Eh, sorry. Our current paramter handlers do nothing, the "parent" method handler is doing the processing actually. We should modify ModelProcessor. We should also modify it to be able to use the same annotation on different element types. |
i quickly hacked it a bit and now it processes the annotations, regardless of thier location (field, method or param) |
its working! :) @Bean
public EmptyDependency dependency;
@Bean
protected void injectDependency(EmptyDependency methodInjectedDependency) {
}
protected void injectDependency2(@Bean EmptyDependency dep) {
} |
this one works too: protected void twoDependencies(@Bean EmptyDependency bean1, @Bean(SomeImplementation.class) SomeInterface bean2) {
} currently this generates one instance of the bean per injection... |
Great work! 👍 Maybe this is the time to open a WIP PR, because we are not specifying the feature, but the implementation. |
doing some rough cleanup before creating PR. some parts of the code are a bit hacky ;) |
Setter injection is implemented. Constructor injection only makes sense in case of beans. If there is need for that, we will open a new issue. |
We currently force the users to have at least package private (default) scopes for their fields. Some users might not appreciate that. We could allow constructor injection / setter injection, this way :
The text was updated successfully, but these errors were encountered: