Thanks Jesse, I was thinking, "maybe it's time for an InjectorBuilder," before I got to that suggestion, so I think we're on the same wavelength here. I was also happy to hear this would be a step toward splitting production stage options, which would be very useful.
The bug report <http://code.google.com/p/google-guice/issues/detail?id=342>suggests people are interested in different levels of help/surprise from Guice, up to requiring all bindings to be explicit. We're probably willing to accept a bit more assistance than this. One possible configuration would be to allow no-arg constructors if a class had no other constructor. Sounds appealing, but I have a bit of a concern over classes that are intended to be constructed through other means (e.g., a static creation method that does interesting initialization, or a Factory/Builder that calls setters before returning the fully initialized product). I'll put some more thought into how the configuration might look, and give others an opportunity to make suggestions, before launching into this. - Russ On Fri, Jun 5, 2009 at 9:08 AM, Jesse Wilson <[email protected]> wrote: > On Wed, Jun 3, 2009 at 7:29 PM, Russ Rufer <[email protected]> wrote: > >> Hi Jesse, >> >> Gmail has been bitten a few times by Guice's choice to use a no-arg >> constructor when there is no @Inject annotation. There have been cases where >> someone didn't realize a collaborator wasn't Guiced already, and injected it >> (but there was, unfortunately, a no-arg constructor that gets used--even >> though it's inappropriate for the place it's injected). >> >> Difficult bugs to find. Ugly post-mortems. >> >> Bad enough that the team would probably like the option to supress the >> automated use of unannotated no-arg constructors. I think more ideal would >> be to indicate in a module that one or more packages (probably recursively) >> should ignore the unannotated no-arg, which would let us still use no-args >> in 3rd party code without providers. >> >> Not sure about all this, but would you support a feature like that? Or >> maybe have an alternative? >> >> I know Guice changed a lot since I was crawling around in it at version >> 1.0, but I might be able to implement something like this without too much >> hand holding. >> >> - Russ >> > > > Excellent! > I agree that we want this feature, and I'd love to let you do the coding! > > I think this idea is totally solid, and it fixes a > bug<http://code.google.com/p/google-guice/issues/detail?id=342> reported > by Bob! So we're all on board. You'll want to disable use of constructors > not annotated "@Inject", unless the class is explicitly mentioned - either > in a single bind statement or in the target clause of a linked/provider key > binding: > bind(Foo.class); // Foo still doesn't need an @Inject constructor > bind(Foo.class).to(FooImpl.class); // neither does FooImpl > bind(Foo.class).toProvider(FooProvider.class); // or FooProvider > > How do we configure this? > This is the biggest problem. Thus far, we don't have much prior-art on how > the Injector's internal rules are changed. > > We have a slight precedent for system properties, but this is only for > mildly supported hacks: > guice.allowNulls.bad.bad.bad: used to suppress failures when @Nullable > is missing > guice.custom.loader: used to turn off OSGi-friendly classloading > I don't think we want to go this route here. > > We could define a full GuiceConfiguration value object, and let the user > bind() that. This is tricky because we'd need to resolve that object > before processing the other bindings. This becomes a problem if the > GuiceConfiguration depends on other bindings: > @Provides GuiceConfiguration provideGuiceConfiguration(DatabaseConnection > d) { ... } > Imagine how poorly defined things get if DatabaseConnection doesn't have an > @Inject-annotated constructor! > > We could add new configuration APIs to binder(): > binder().useConfiguration(new GuiceConfiguration(...)); > This is concise and well-defined, but it means you've got a new Element > type that needs to be wired into about ten different places. > > We could do something on the Guice.createInjector() factory method: > Guice.createInjector(new GuiceConfiguration(...), new MyModule()); > This is nice because it lets the Injector-creator specify configuration, > rather than imposing it on any individual module. The only cost is that you > now need two new factory methods on Guice.createInjector(). > > We could create a public InjectorBuilder class, that permits configuration. > It would be an alternative to the Guice.createInjector() API: > Injector injector = new InjectorBuilder() > .production() > .requireInjectAnnotation() > .add(new MyModule()) > .add(moreModules) > .build(); > I dig this, especially since it opens up the door to even more > configuration options. For example, we've long wanted to split the > production stage into separate aspects: eager loading and error detection. > > I think the builder is my preferred option. I'm interested to hear what > others have to say on this. > > Advice for hacking on Guice: > The codebase is complex, but not that complex and I've tried reasonably > hard to Javadoc implementation details. Be sure to first familiarize > yourself with the Guice > internals<http://code.google.com/docreader/#p=google-guice&s=google-guice&t=Internals>docs > (which you'll need to update after your change). > > If you see something that could be easily improved, improve it! Including > internal documentation. There's a very small number of places where we do > weird things for a performance benefit - like using arrays instead of lists. > If you get the urge to fix this stuff, don't! But if it's weird and > undocumented, I suspect we're missing an "// this is for performance" note. > > Once you've written and tested the change (I predict you'll need to change > 5 or 6 files), please email me a patch. I'll review it, and then you can > submit it from your own desktop. > > Lookin' forward to a patch! > -Jesse > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "google-guice-dev" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en -~----------~----~----~----~------~----~------~--~---
