frjaeger220 / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

AssistedInject: Multi-constructor injection #430

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
With toConstructor bindings in the code base, I've started thinking about
multi-constructor assisted inject, i.e. factories that allow you to have
more than one create-method per type.

========

Example:

class Car {
  public Car(Frame frame, @Assisted Gas gas) {...}
  public Car(Frame frame, @Assisted Electricity electricity) {...}
}

interface CarFactory {
  Car createGasCar(Gas gas);
  Car createElectricCar(Electricity electricity);
}

=========

While this seems rather straight-forward, there are two issues I'd like to
discuss before I start the work. They both impact which constructor is
selected for injection:

1. (Class-) target selection: If a return type A is bound to type B for the
factory, and B is bound to C in the injector, the factory will attempt to
return a C when asked for an A. Is this the desired behaviour or do we want
to ignore C and just look at constructors from B? What happens if no type B
is bound for the factory but A is bound to C in the injector (we currently
return a C)? I suggest just looking at constructors of B (in the first
case) or A (second case).

2. Constructor selection: I'd use the following algorithm to find the
constructor in the target class but would like your input, especially if
you think it's wrong:

1. Ignore @Inject constructors.
2. Iterate through constructors, matching factory method parameters
in-order to @Assisted constructor parameters. If match, return.
3. Iterate through constructors, matching factory method parameters
set-like to @Assisted constructor parameters. If single match, return.
4. Give up.

Let me know what you think!

Original issue reported on code.google.com by aragos on 23 Sep 2009 at 9:10

GoogleCodeExporter commented 9 years ago
Spent some time doing this today.  For the most part it's doable without any 
changes
in core Guice, but there's two methods that can be added that would make it 
much much
nicer. 

 The first is InjectionPoint.isInjectableConstructor, which is basically a portion of
the current InjectionPointer.forConstructor(TypeLiteral) -- it just returns 
true if
the constructor's injectable.  Without that method, things get a little 
confusing. 
Of course, if we remove the need to have any @Inject on the assisted class, 
there's
no need for this method.  I went with keeping @Inject on any potential 
constructors,
but there's definitely no requirement for it.  The patch would need minor 
tweaking to
support any constructor (so long as it had @Assisted parameters).  

 The second is Injector.hasBinding.  This is an analogue to Injector.getBinding,
except it will only return true or false instead of trying to create a JIT 
binding. 
The idea behind this is that because FactoryProvider2 now lets you use linked
bindings across Injectors, it's necessary to see if the binding for the return 
type
already exists in the injector.  You can only do this after initialization 
(after the
Injector is created).  Without doing this, the first two tests in
FactoryModuleBuilderTest fail.  There are other ways of doing it without the new
method, such as calling Injector.getAllBindings on the injector (and each of its
parent injectors), but that creates copies & seems very heavyweight IMO.  
hasBinding
seems like a useful method in its own right, anyway.

Aside from that, the changes are localized all to FactoryProvider2.

This patch requires issue 434 is fixed, also.

If you'd like to tweak the logic for choosing a constructor, it should be pretty
straightforward given the patch's structure.

Original comment by sberlin on 5 Oct 2009 at 5:38

Attachments:

GoogleCodeExporter commented 9 years ago
fixed with r1148. from commit log: patch AssistedInject extension to support
multi-constructor injection with FactoryModuleBuilder.  reuses @AssistedInject
annotation to mark valid constructors.  undeprecates @AssistedInject.  
deprecates
FactoryProvider in
favor of FactoryModuleBuilder now that all functionality is matched and
performance problems are fixed.  you can still get the "old" behavior (ordered
matching of parameters, non-guiced objects, etc..) by using
@AssistedInject+FactoryProvider, but using @Inject or @AssistedInject with
FactoryModuleBuilder will give you the new behavior.

Original comment by sberlin on 27 Mar 2010 at 8:37