-1

My problem is here; I don't know if i can do that with guice:

public class Foo {
 private final Bar bar;

 @Inject
 public Foo(Injector injector) {
  this.bar = new Bar();
  injector.injectMembers(bar);
 }
}

*bar will be injected correctly or not ? Thank you

Romain-p
  • 518
  • 2
  • 9
  • 26

1 Answers1

3

Yes, this should work. It's OK for the process of constructing one instance to cause other injections to occur (in fact, this is the way dependency injection works normally).

A probably better alternative for the same effect would be:

@Inject Foo(Provider<Bar> barProvider) {
  this.bar = barProvider.get();
}

Since this will cause Guice to construct the Bar instance and inject its fields.

As an aside, this example demonstrates some bad practices:

  • It's bad form to make @Inject constructors public. The whole point of dependency injection is to make it so that clients of your class don't need to know about your class's dependencies. That's subverted if they ever construct the class directly. If you really want a "default" constructor, just provide a non-@Inject constructor or a factory method (either on the class or elsewhere in the package).
  • injectMembers is usually a best avoided if possible. Wherever possible, prefer constructor injection. Among other things, this is necessary if you're going to be sharing instances across threads, since injectMembers may not guarantee safe publication. Like any post-constructor initialization step, it also makes class invariants harder to reason about.
  • As you've probably inferred, injecting the Injector is a code smell. Outside of really heavy metaprogramming or framework code, it should almost never be necessary. Like reflection, it can be a great way to build "magic" functionality, but also like reflection, it can also cause extremely weird runtime errors if you do it wrong.
Daniel Pryden
  • 59,486
  • 16
  • 97
  • 135
  • Thank you. But my example is not really my situation. I really need to inject a field which'sn't able to be created by a provider. – Romain-p Mar 14 '15 at 23:33
  • @Romain-p: why can't you use a `Provider` here? Can you provide more code to demonstrate what's going on? There may be other techniques (e.g. possibly you could extend the Guice binding DSL with your own methods for building bindings) that would make this approach less fragile. – Daniel Pryden Mar 14 '15 at 23:37
  • I can show you the code if you have skype (by screen share). – Romain-p Mar 14 '15 at 23:41
  • @Romain-p: I don't really want to see your code, just a sketch of why you need to do it this way. Could you put an example (suitably anonymized) in a gist or pastebin? (Or better yet, an anonymized/redacted version on codereview.stackexchange.com?) It's possible that you really do need field injection, but it seems likely that there's a better way. – Daniel Pryden Mar 14 '15 at 23:55
  • I need to inject getAction() (Command): https://gist.github.com/Romain-P/8fcaeda4fa075ff8ab21 – Romain-p Mar 15 '15 at 00:09
  • 2
    +1, though if `Provider` provides exactly one instance within the constructor, just inject `Bar` instead. Great answer! – Jeff Bowman Mar 15 '15 at 01:08