Open GoogleCodeExporter opened 9 years ago
fixed in r1141 (using Guice.createInjectorBuilder)
Original comment by sberlin
on 11 Feb 2010 at 10:12
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
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
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
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
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
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
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
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
r1142 should clean this up now. new InjectorBuilder() is the new
Guice.createInjectorBuilder.
Original comment by sberlin
on 13 Feb 2010 at 4:19
Methods on InjectorBuilder should probably be public. =P
Original comment by cgdec...@gmail.com
on 21 Feb 2010 at 7:44
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
re-closing as fixed based on new suggestions, from commits r41141, r1142, &
r1143.
Original comment by sberlin
on 22 Feb 2010 at 7:53
Original issue reported on code.google.com by
limpbizkit
on 22 Jun 2009 at 6:22