vipx / google-guice

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

Guice Assisted Inject with multiple implementations of interface #809

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I have an interface A with multiple implementations say B, C and D. I am using 
guice assisted inject to create these classes. Classes B and C use the same 
assister parameters(say b, c) in constructor while D has one different 
parameter(say d). The way I implementing this is have a factory AFactory with 2 
methods: create(int b, int c) and createD(int d) In my module, I have created a 
PrivateModule to bind the concrete class to factory.

The code looks like this:

public static PrivateModule getAFactoryModule(final String 
factoryBindingKey,final Class<? extends A> targetClassToCreate) {
    return new PrivateModule() {

        @Override
        public void configure() {
            install(new FactoryModuleBuilder().implement(
                    A.class, targetClassToCreate).build(
                    AFactory.class));

            bind(AFactory.class).annotatedWith(Names.named(factoryBindingKey)).to(
                    Key.get(AFactory.class));
            expose(Key.get(AFactory.class, Names.named(factoryBindingKey)));
        }
    };
}
I call PrivateModule like this:

install(getAFactoryModule("B", B.class));
install(getAFactoryModule("C", C.class));
install(getAFactoryModule("D", D.class));
But, this gives an error saying:

com.guice.test.B has @AssistedInject constructors but none of them match the 
parameters in method com.guice.test.AFactory.createD(). Unable to create 
assisted inject

com.guice.test.C has @AssistedInject constructors but none of them match the 
parameters in method com.guice.test.AFactory.createD(). Unable to create 
assisted inject

com.guice.test.D has @AssistedInject constructors but none of them match the 
parameters in method com.guice.test.AFactory.create(). Unable to create 
assisted inject
It seems Guice is trying to use different create methods than what is expected. 
Any idea how this can be resolved? Any pointers would be appreciated!

Thanks!

Original issue reported on code.google.com by nishantm...@gmail.com on 4 Jun 2014 at 2:44

GoogleCodeExporter commented 9 years ago
When implementing the factory, assistedinject has to be able to match *every* 
method it sees.  You're installing the factory 4 times, and in each it's saying 
it doesn't know how to implement one of the methods in the factory.

Do you actually need B & C to be using the exact same method?  The easiest way 
to do this is to do something like:
  interface Factory {
    @Named("A") Foo createA();
    @Named("B") Foo createB(int a, int b);
    @Named("C") Foo createC(int a, int b);
    @Named("D") Foo createD(int a);
  }

and then install it using:

 install(new FactoryModuleBuilder()
    .implement(Keys.get(Foo.class, Names.named("A")), A.class)
    .implement(Keys.get(Foo.class, Names.named("B")), B.class)
    .implement(Keys.get(Foo.class, Names.named("C")), C.class)
    .implement(Keys.get(Foo.class, Names.named("D")), D.class)
    .build(AFactory.class));

.. and not mess with PrivateModules at all.  Doing it this way will associate 
each 'create' method with a different implementation.

Original comment by sberlin on 4 Jun 2014 at 2:58

GoogleCodeExporter commented 9 years ago
Thanks for your response!

The approach you told would work and I thought about it too. But, I want to use 
same create method for getting instance of B and C, which implementation of A 
(B, C or D) to return should be determined by Guice bindings.

If I create separate methods, then in my application code, I need to have a if 
else and invoke one of the factory's create method. Having this if-else means 
that the application code has to somehow know which implementation of A it 
needs. I want to avoid this if-else.

Is that possible?

Original comment by nishantm...@gmail.com on 4 Jun 2014 at 3:12

GoogleCodeExporter commented 9 years ago
A few options:

1) Install the factory as in comment #1, but introduce a shim that delegates 
D->D and the other to either B or C.  Users will inject the shim, and the shim 
will hide away the fact that the factory has two different methods for B & C.

or 2) Install the factory as mix of comment #1 & the original way.  The 
difference would be you'd only have two installations: one for B & one for C, 
both of which would also be able to create D.

I'd prefer (1) because it's a bit more straightforward -- the factory can 
create everything, and explicit code is deciding how it's forwarding to one or 
another.

Original comment by sberlin on 4 Jun 2014 at 3:19

GoogleCodeExporter commented 9 years ago
Option 1 seems to be like a factory over the factory interface. The new factory 
decides which factory interface method to call based on the shim.

Option 2 is not quite clear to me. How will installations for B and C know how 
to create D? Can you please elaborate this one?

Original comment by nishantm...@gmail.com on 4 Jun 2014 at 3:45

GoogleCodeExporter commented 9 years ago
Correct, (1) turns the original factory into a delegate and the shim is the 
real factory (delegating to the delegate).  The delegate's really just an 
implementation detail, though, and the interface could be package-private, etc.

(2) would be done with something like:
  interface Factory {
    @Named("BC") A create(int foo, int bar);
    @Named("D") A create();
  }
  Module extends PrivateModule {
    @Override public void configure() {
      install(new FactoryModuleBuilder()
          implement(Key.get(A.class, Names.named("BC"), targetBCClazz)
          implement(Key.get(A.class, Names.named("D"), targetDClazz)
          build(AFactory.class));
          // + bind to named type, expose
    }
  }
  install(getAFactoryModule("B", B.class, D.class));
  install(getAFactoryModule("C", C.class, D.class));

Original comment by sberlin on 4 Jun 2014 at 3:54

GoogleCodeExporter commented 9 years ago
Thanks!
Option 1 would definitely work. I tried option 2, but that doesn't work.

I have getting errors while injection:
Factory method [public static com.google.inject.Injector 
com.google.inject.Guice.createInjector(java.lang.Iterable)] threw exception; 
nested exception is com.google.inject.CreationException: Guice creation errors:

1) No implementation for com.guice.test.AFactory was bound.

2) com.guice.test.A is an interface, not a concrete class.  Unable to create 
AssistedInject factory.
 while locating com.guice.test.A at com.guice.test.AFactory.create

3) com.guice.test.A is an interface, not a concrete class.  Unable to create 
AssistedInject factory.
while locating com.guice.test.A at com.guice.test.AFactory.create

My guess is that the names used for binding is different while installing 
FactoryModule("BC"/"D") and while binding the factory after installing 
("B"/"C"), but I am not sure.

Can you help me understand what might be going wrong?

Original comment by nishantm...@gmail.com on 4 Jun 2014 at 4:14

GoogleCodeExporter commented 9 years ago
It's hard to know exactly what's going wrong without seeing the whole testcase. 
 StackOverflow (or the user mailing list) might be a better place to continue 
the discussion.

Original comment by sberlin on 4 Jun 2014 at 5:03

GoogleCodeExporter commented 9 years ago
Ok - thanks for your response.

I would prefer to go with approach 1, the problem I see with second approach is 
that it requires to unnecessarily create D along with B and C and this would 
grow with new implementations.
Also, in case I need another implementation of A which requires different 
parameters than B/C or D then this approach will get messy.

If there are no better ways, I will go with option 1.

Thanks!

Original comment by nishantm...@gmail.com on 5 Jun 2014 at 1:05