Injection and inheritance

3,124 views
Skip to first unread message

Cédric Beust ♔

unread,
Feb 8, 2013, 8:47:58 AM2/8/13
to google...@googlegroups.com
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

Fred Faber

unread,
Feb 8, 2013, 9:09:31 AM2/8/13
to google...@googlegroups.com
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

--
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...@googlegroups.com.
To post to this group, send email to google...@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.
 
 

Cédric Beust ♔

unread,
Feb 8, 2013, 9:21:41 AM2/8/13
to google...@googlegroups.com
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

Tim Boudreau

unread,
Mar 10, 2013, 9:31:44 PM3/10/13
to google...@googlegroups.com, ced...@beust.com
I've run into the same issue, and every time I've looked at AssistedInject it seemed boilerplate-heavy and kludgey.  It also doesn't scale well code-wise if you either have a lot of things you'll want to inject that way, or if you want to write a framework that allows its users to provide a list of injectable types and pass in objects to inject.

If what you're writing has some sort of natural request/response scope, the generic solution I've used is custom scopes.  It doesn't change the fact that *something* at the front of your call stack has to know how to derive the value of "name", but that's true of AssistedInject too, so the idea that it's more decoupled one way than the other is an illusion - it's just a question of, would you rather write the boilerplate of assisted inject or 

class OuterThing {
  private final ScopeRunner runner;
  @Inject
  OuterThing (ScopeRunner runner) {
      this.runner = runner;
  }

  public void onRequest () {
    String name = ...
    try (AutoCloseable ac = scope.enter(name, otherThing, whatever)) {
       runner.run (R.class);
    }
  }

  private static class R implements Runnable {
     private final Dep dep;
     @Inject R(Dep dep) { this.dep = dep; }
     public void run() { ... }
  }
}

The more "live" things you have that can usefully be injected, the more this approach pays off.  Yes, something has to touch the injector post-startup, but that's happening anyway with AssistedInject, and the real value is in having a clean programming model.


-Tim

Reply all
Reply to author
Forward
0 new messages