google / guice

Guice (pronounced 'juice') is a lightweight dependency injection framework for Java 11 and above, brought to you by Google.
https://github.com/google/guice
Apache License 2.0
12.5k stars 1.67k forks source link

Allow Binding annotations to be used as dependants in providers #1098

Open Groostav opened 7 years ago

Groostav commented 7 years ago

As a guice-user looking to use interfaces wherever possible I would like a convenient way to specify more specific types than my constructor (or field) signature allows for, so that I might more easily express my dependencies.

Hey guys,

so we have a use case that I don't believe is really uncommon. The gist is that we have a factory that has a dependency on a bunch of providers. It exposes a single method that switches based on its input and the state of the application to select one of these providers to use. The values given by these providers could be specific, but they would rather use an an interface that is the interface on this factory method.

some code, given that I have:

public class DomainKnowledgeSwitchingFactory{
  //...

  @Inject
  DomainKnowledgeSwitchingFactory(
    Provider<DomainInterfaceType> scenarioOneHandlerProvider,
    Provider<DomainInterfaceType> scenarioTwoHandlerProvider,
    OtherRuntimeDeps otherRuntimeDepsPossiblyAssistedInjectProvided
  ) //{ ... }

  public DomainInterfaceType makeDomainInterfaceTypeForCurrentSItuation(CurrentSituation situation){
    if(complexDomainLogicThatDerrivesScenarioOne(situation)){
      return scenarioOneHandlerProvider.get()
    }
    else if (complexDomainLogicForScenarioTwo(situation)){
      return scenarioTwoHandlerProvider.get()
    }
    else { throwOrExecuteSomeDefault() }
  }
}

I think it should be pretty apparent that I don't want scenarioOneHandlerProvider to provide the same type as scenarioTwoHandlerProvider. So, given that I want to return different types for scenario one and scenario two, these are my options:

existing option one

request the most specific type at provider-injection:

-    Provider<DomainInterfaceType> scenarioOneHandlerProvider,
+    Provider<SOneHandlerImplementsDomainInterface> scenarioOneHandlerProvider,
-    Provider<DomainInterfaceType> scenarioTwoHandlerProvider,
+    Provider<STwoHandlerImplementsDomainInterface> scenarioTwoHandlerProvider,

downside:

Testing this is harder than it needs to be. Nothing about this code has a runtime dependency on SOneHandlerImplementsDomainInterface, yet in testing I'm now going to have to create a hard instance of that type, even though this code compiles and expresses its intent through variable names with the previous type. Yes I'm aware of mocking frameworks, but creating an instance of a concrete type is always going to be harder than creating an instance of the interface for testing!

existing option two

use binding annotations:

-    Provider<DomainInterfaceType> scenarioOneHandlerProvider,
+    @Named("ScenarioOne") Provider<DomainInterfaceType> scenarioOneHandlerProvider,
-    Provider<DomainInterfaceType> scenarioTwoHandlerProvider,
+    @Named("ScenaroTwo") Provider<DomainInterfaceType> scenarioTwoHandlerProvider,

then update your module bindings:

bind(DomainInterfaceType.class).annotatedWith(Names.named("ScenarioOne")).to(SOneHandlerImplementsDomainInterface.class)
bind(DomainInterfaceType.class).annotatedWith(Names.named("ScenarioTwo")).to(STwoHandlerImplementsDomainInterface.class)

downside:

In my production code I'm actually injecting 5 such providers and soon there will be a 6th. This means the calling module(s) will each need 5 separate bindings. With one growing each time. To a user accustomed to spring or other explcit-configuration dependency injection systems this might be OK, but to those of us who work hard to keep things auto-wired nicely, its a pain in the butt.

proposed new option

increase flexibility of providers: I was hoping to write something like

-    Provider<DomainInterfaceType> scenarioOneHandlerProvider,
+    @ActualType(SOneHandlerImplementsDomainInterface.class) Provider<DomainInterfaceType> scenarioOneHandlerProvider,
-    Provider<DomainInterfaceType> scenarioTwoHandlerProvider,
+    @ActualType(SOneHandlerImplementsDomainInterface.class) Provider<DomainInterfaceType> scenarioTwoHandlerProvider,

Now this could be added as another special case pre-supplied BindingAnnotation, or it might (more generally) register annotation instances as available to providers, while also lifting provides from implementation providers to type-alias providers:

bind(DomainInterfaceType.class)
    .annotatedWith(ActualType.class)
    .to((ActualType annotationInstance) -> { annotationInstance.value })

or perhalps with a similar provider method:

@ProvidesAlias DomainInterfaceType makeDomainInterfaceType(ActualType actualType) {
  return actualType.value
}

many thanks for a great library and keep up the good work!

dimo414 commented 7 years ago

At a glance both the status quo and option one seem more reasonable than an @ActualType annotation. I'd try to go the other direction, and remove the implied expectation that scenarioOneHandlerProvider and scenarioTwoHandlerProvider return specific implementations. It's hard to say for certain without understanding complexDomainLogicThatDerrivesScenarioOne(), but that's the part that seems smelly to me.