InjectorBuilder

[GoogleCodeExporter]

It would be handy to create an injector using an API more capable than 
"Stage.PRODUCTION" or 
"Stage.DEVELOPMENT". For instance:

Injector injector = new InjectorBuilder()
    .eagerlyLoadSingletons()
    .forbidJustInTimeBindings()
    .addModules(modules)
    .build();

Unfortunately, this would give us two ways to create injectors (the builder, or 
Guice.createInjector), 
which makes our API clumsier.

Original issue reported on code.google.com by limpbizkit on 22 Jun 2009 at 6:22

[GoogleCodeExporter]

fixed in r1141 (using Guice.createInjectorBuilder)

Original comment by sberlin on 11 Feb 2010 at 10:12

• Changed state: Fixed

[GoogleCodeExporter]

I'm not crazy about this API. Does it really carry its weight? I think I went 
with a static 
factory in the first place because we don't know for sure if Module instances 
are 
reusable. Can't we just put this methods on Binder?

Original comment by crazybob...@gmail.com on 11 Feb 2010 at 10:31

• Changed state: New

[GoogleCodeExporter]

I think both the builder pattern & the Binder/Module pattern solve the "method
overloading sucks" issue, but putting methods on Binder opens up more issues 
than it
solves.  For example, what happens if one Module calls stage(Stage.PRODUCTION)
whereas another module calls stage(Stage.DEVELOPMENT) (or substitute stage with 
any
other future injector building method that takes a parameter)?  The only sane 
thing
to do would be to throw an error.  The builder API does a nice runaround this 
by only
letting the builder of the Injector decide how it's built.

(I'm not 100% sure I understand the "Module instances are reusable" comment, so 
my
comments may not directly address what you're asking.)

Original comment by sberlin on 11 Feb 2010 at 10:37

[GoogleCodeExporter]

Technically, you could call stage() twice on the builder, too, but I understand 
where 
you're coming from.

I just don't want to expand the API. If we decide this is the only way, 
InjectorBuilder 
should be a separate class, and we should deprecate the Guice class.

Original comment by crazybob...@gmail.com on 11 Feb 2010 at 10:43

[GoogleCodeExporter]

True, you could definitely call stage() twice on the builder -- but atleast the
builder isn't distributed among different classes/authors, so you see the error 
in
front of you.  That's the big pitfall of Binder, I think -- it scales *so* well 
that
it even scales to different authors & classes, which is where these building 
options
fail to make sense.

Agreed on separating InjectorBuilder.  Guice exposes it right now because
InjectorBuilderImpl needs to be in internal (because it uses package-private 
code). 
If Guice is deprecated, InjectorBuilder can become a class (right now it's an
interface) and delegate to the internal InjectorBuilderImpl (as opposed to
InjectorBuilderImpl implementing InjectorBuilder right now).

Original comment by sberlin on 11 Feb 2010 at 10:50

[GoogleCodeExporter]

I like a new public type InjectorBuilder with a no args constructor:

  Injector injector = new InjectorBuilder()
    .forbidJustInTimeBindings()
    .build();

Or perhaps, an inner class of the Injector interface:

  Injector injector = new Injector.Builder()
    .forbidJustInTimeBindings()
    .build();

Original comment by limpbizkit on 12 Feb 2010 at 5:45

[GoogleCodeExporter]

What do you think should be done about Injector.createChildInjectorBuilder?  
Scrap it
entirely (always inherit values of parent & don't expose a builder to allow 
them to
change), allow all the configurability of a parent InjectorBuilder (reuse the 
same
class), or allow a subset of configurability (use a new class specifically for 
child
builders)?

Original comment by sberlin on 12 Feb 2010 at 1:14

[GoogleCodeExporter]

Hmmmm. . . . good question.

I'm tempted to just limit child injectors to the same configuration options as 
their 
parent. It simplifies things!

Original comment by limpbizkit on 12 Feb 2010 at 5:08

[GoogleCodeExporter]

Alright, I'll clean this up over the weekend.  Thanks for the ideas/input!

Original comment by sberlin on 12 Feb 2010 at 11:59

[GoogleCodeExporter]

r1142 should clean this up now.  new InjectorBuilder() is the new
Guice.createInjectorBuilder.

Original comment by sberlin on 13 Feb 2010 at 4:19

[GoogleCodeExporter]

Methods on InjectorBuilder should probably be public. =P

Original comment by cgdec...@gmail.com on 21 Feb 2010 at 7:44

[GoogleCodeExporter]

Ha!  That's what I get for blindly changing from an interface (inheriting the 
method
visibility from the interface's visibility) to a class.

fixed in r1143.

Original comment by sberlin on 22 Feb 2010 at 1:31

[GoogleCodeExporter]

re-closing as fixed based on new suggestions, from commits r41141, r1142, & 
r1143.  

Original comment by sberlin on 22 Feb 2010 at 7:53

• Changed state: Fixed