@IntoSet analog for injectable classes
laneb opened this issue · 3 comments
I have a use case in which I have N injectable services that implement an interface, and I want to inject all such services at another site. Like
interface ServiceInterface
class Service1 @Inject internal constructor(...): ServiceInterface
class Service2 @Inject internal constructor(...): ServiceInterface
...
class ServiceN @Inject internal constructor(...): ServiceInterface
class App @Inject internal constructor(
private val services: Set<ServiceInterface>,
)
@Component
interface AppComponent {
fun app(): App
}
The best approach I have found to making this set injectable is using multibindings along the lines suggested in this SO thread. This requires boilerplate in the form of binding for each service - something like
@Module
interface ServiceModule {
@Bind
@IntoSet
fun service1(service: Service1): ServiceInterface
@Bind
@IntoSet
fun service2(service: Service2): ServiceInterface
...
@Bind
@IntoSet
fun serviceN(service: ServiceN): ServiceInterface
}
@Component(
modules = [
ServiceModule::class
]
)
interface AppComponent {
fun app(): App
}
That's a lot of boilerplate! Especially considering that part of my motivation for using DI is to reduce boilerplate. I would like to inject these services into the set without needing to enumerate all such services - i.e. an appropriately annotated service is discovered for inclusion in the set automagically. It seems to me (being newish to dagger) that it should be possible to annotate the services for set injection directly and do away with the binding declarations. Along the lines of
class ServiceN @Inject @IntoSet(ServiceInterface::class) internal constructor(...): ServiceInterface
Would such a pattern be feasible?
@JakeWharton Thanks for the links! Good to know I'm not alone in wanting such a feature :^)
It seems like #2026 is the most thorough discussion of the limitations and implications that would make such a feature unfeasible; however, I think the approach I suggest mitigates those concerns. I would like to press this a bit, because a feature like I describe would make Dagger significantly less annoying for my use case. If this discussion would be more welcome in one of the linked tickets, please let me know.
Particularly
There's no way for Dagger to know that your type should be added to the set in its normal traversal.
I did anticipate this - I assume it's complicated and prohibitively slow to search the class path for all subtypes of the injected type. I'm sure that injecting Set<Any> without any constraint on which Any you're interested in would be a fun corner case. This is why I suggest constraining the @IntoSet or equivalent annotation with the specific type of the collection to be injected (IntoSet(::ServiceInterface) in my example). In fact, I see this idea mentioned briefly in #2026
Dagger would either need to ... allow you to pass that information in via the annotation (which doesn't work well with parameterized types)
For my use case, at least, this is a non-issue. And if I did really need to inject a parameterized type, there are workarounds as long as I'm willing to shape my code for Dagger's better understanding, which I am. For example, if we rework the example like
interface ServiceInterface<Result>
interface A
interface A1: A
interface A2: A
interface B
class ServiceA1 @Inject internal constructor(...): ServiceInterface<A1>
class ServiceA2 @Inject internal constructor(...): ServiceInterface<A2>
class ServiceB @Inject internal constructor(...): ServiceInterface<B>
and I want to inject a Set<ServiceInterface<A>> into my application, I could add an intermediate interface, along the lines of
interface AServiceInterface: ServiceInterface<A>
class ServiceA1 @Inject @IntoSet<AServiceInterface::class> internal constructor(...): AServiceInterface
class ServiceA2 @Inject @IntoSet<AServiceInterface::class> internal constructor(...): AServiceInterface
class App @Inject internal constructor(
private val services: Set<@JvmSuppressWildcards AServiceInterface<*>>,
)
Not ideal, but certainly workable and significantly preferable to enumerating the service implementations in a module.
As for the other concerns raised
There's no way to remove the type from the set. For example, you still want to inject Set but you don't want that particular class in the set. With modules you just remove the module, but with this solution you would have to remove the class from your class path, which can either be difficult or impossible if it's used elsewhere.
One of the useful features of multibindings is that the set can change depending on which component you install the module int. With this solution, you don't choose a component so it's ambiguous where Dagger should install it.
Acknowledged that the modules provide more fine-grained control over what instances are included in the Set, but as I have observed the tradeoff is more boilerplate. It seems to me there is room for both approaches.
I did anticipate this - I assume it's complicated and prohibitively slow to search the class path for all subtypes of the injected type. I'm sure that injecting Set without any constraint on which Any you're interested in would be a fun corner case. This is why I suggest constraining the @IntoSet or equivalent annotation with the specific type of the collection to be injected (IntoSet(::ServiceInterface) in my example).
It isn't just a matter of constraining. Searching the classpath is just something Dagger doesn't do and probably isn't something we want it to do in the future. This basically limits the solution space to Hilt where we could generate a Dagger module.
Not ideal, but certainly workable and significantly preferable to enumerating the service implementations in a module.
The issues with things like qualifiers and parameterized types are things you can workaround, if you know about them. But that is actually the main issue. I don't want to create a new API for doing something just to save on boilerplate, but then in addition to the cognitive overhead of having two ways to do something, you also have the overhead of learning certain features don't work with other features. IMO, it isn't worth the tradeoff as we already know it is difficult for people to get up to speed on Dagger features and there is already a fair amount of multiple ways to do the same thing that adds confusion.
I feel like if we're going to add the cognitive overhead cost of doing something multiple ways, the other way has to be pretty clean and intuitive and cover a lot of cases. So far, I just haven't seen anything or come up with anything that meets that. Closing this since #2026 (comment) I think also covers a lot of the current thoughts on this.
Separate aside:
Separately, I'm going to assume your example falls into this other issue because it is just an example and that this probably doesn't apply to you, but just for others who may be reading this since I have seen this in code before, if you have a single module that defines ALL of your set contributions, you don't need multibindings.
@Module
interface ServiceModule {
@Bind
@IntoSet
fun service1(service: Service1): ServiceInterface
@Bind
@IntoSet
fun service2(service: Service2): ServiceInterface
...
@Bind
@IntoSet
fun serviceN(service: ServiceN): ServiceInterface
}
Should just be rewritten as
@Module
interface ServiceModule {
@Provides
fun services(service1: Service1, service2: Service2, ..., serviceN: ServiceN): Set<ServiceInterface> {
return setOf(service1, service2, ..., serviceN)
}
}
You only need multibindings if you need to make contributions from multiple sites where no single piece of code knows what all of the contributions should be.