Actually, the third option (field injection) is a very decent approach. I had ruled it out because I didn't think it would work (usually, if I write the code of a constructor, I'm in charge of constructing that class so I can't use @Inject on its fields). However, I tested and it does work, probably due to the fact that this constructor is being invoked by Guice through assisted injection (so I'm guessing that Guice instantiates the class with the factory and then makes a second pass of injectMembers on it?).
With this approach, the base class can add and remove dependencies with zero impact on the subclasses, which is a big plus. The subclass boiler plate issue remains, sadly. Thanks, Fred. -- Cédric -- Cédric On Fri, Feb 8, 2013 at 6:09 AM, Fred Faber <ffa...@faiser.com> wrote: > The short answer is that there's no silver bullet here :/ > > What you've written as a first solution is obvious and functional, and > that's good. But as you highlight there is a nontrivial cost of extension. > > The second solution you've sketched is something I recommend avoiding, > mainly due to the requirement of calling Injector.getInstance(). The > pitfall of it is that you sacrifice up-front checking of required bindings, > since Guice won't know what Key you're requesting until runtime. > > A third solution is to use member injection on the base class, but that > suffers from a generally awkward design and the usual downsides of member > injection. > > A fourth solution, and I've seen gain popularity in practice, is to define > a struct to hold the dependencies of the base class. This gives stability > to your constructors and allows for extension of dependencies: > > public static final BaseDependencies { > private final Dep1 dep1; > private final Dep1 dep2; > ... > private final Dep1 dep; > > @Inject BaseDependencies(...) > } > > public class Base { > protected Base(BaseDependencies deps, String name) { > ... > } > } > > public class A extends Base { > @Inject A(BaseDependencies deps, @UsedAsName String name) { > super(deps, name); > } > } > > All in all I think this pattern of inheritance is not all too frequent, > probably because of the tendency to rely on composition. I mention that > because the fourth imperfect solution seems to be "good enough" to make it > through the day and move one, and so at least I'm not aware of many > requests to improve on the situation. If you've got any ideas though I'd > be curious to hear what other folks might be thinking about for an > improvement. > > Fred > > On Fri, Feb 8, 2013 at 5:47 AM, Cédric Beust ♔ <ced...@beust.com> wrote: > >> I'm trying to find the best way to inject into an inheritance hierarchy. >> >> The base class takes one "live" parameter ("name", not injectable) and a >> bunch of injected dependencies, "dep1", "dep2", etc. The most obvious >> solution is to use assisted injection: >> >> public class Base { >> private Dep1 dep1; >> private String name; >> >> protected Base(String name, Dep1 dep1) { >> this.name = name; >> this.dep1 = dep1; >> } >> } >> >> public class A extends Base { >> public interface IFactory { >> A create(@Assisted("name") String name); >> } >> >> @Inject >> private A(String name, Dep1 dep1) { >> super(name, dep1); >> } >> } >> >> Two things bother me with this approach: >> >> 1. It's a lot of boiler plate. Each new subclass needs to create its >> own factory, its own constructor with all the dependencies, call super >> with >> all these dependencies and add itself to the FactoryModuleBuilder. >> >> 2. If I add a new dependency in the base class, I need to modify all >> the subclasses (add a parameter to their constructor and to their super() >> call). >> >> I considered passing the injector as the sole dependency to the base >> class and let the base class do its own look ups: >> >> public class Base { >> private Dep1 dep1; >> private String name; >> >> protected Base(String name, Injector inj) { >> this.name = name; >> this.dep1 = inj.getInstance(Dep1.class); >> } >> } >> >> public class A extends Base { >> public interface IFactory { >> A create(@Assisted("name") String name); >> } >> >> @Inject >> public A(String name, Injector inj) { >> super(name, inj); >> } >> >> This addresses point 2: when a new dependency is added to the base class, >> only the base class needs to be modified, but it's still awkward to do >> these look ups manually (and injectMembers() doesn't work since we're in >> the constructor). >> >> This still doesn't address point 1: there's still a lot of boiler plate >> involved for each subclass. >> >> Is there a better way? >> >> If not, can we discuss this as a potential new feature? >> >> -- >> Cédric >> >> -- >> You received this message because you are subscribed to the Google Groups >> "google-guice" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to google-guice+unsubscr...@googlegroups.com. >> To post to this group, send email to google-guice@googlegroups.com. >> Visit this group at http://groups.google.com/group/google-guice?hl=en. >> For more options, visit https://groups.google.com/groups/opt_out. >> >> >> > > -- > You received this message because you are subscribed to the Google Groups > "google-guice" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to google-guice+unsubscr...@googlegroups.com. > To post to this group, send email to google-guice@googlegroups.com. > Visit this group at http://groups.google.com/group/google-guice?hl=en. > For more options, visit https://groups.google.com/groups/opt_out. > > > -- You received this message because you are subscribed to the Google Groups "google-guice" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-guice+unsubscr...@googlegroups.com. To post to this group, send email to google-guice@googlegroups.com. Visit this group at http://groups.google.com/group/google-guice?hl=en. For more options, visit https://groups.google.com/groups/opt_out.