On Mon, Oct 6, 2008 at 5:11 PM, James Strachan <[EMAIL PROTECTED]>wrote:
> > 2008/10/6 Robbie Vanbrabant <[EMAIL PROTECTED]>: > > I love your efforts, but I wonder if we can have something that has less > > impact on core Guice. > > Would it be possible to create like a Scope decorator that adds lifecycle > > support to a scope? > > I tried to go down this route; but its a tad messy as we'd have to > stop using the standard Guice scopes. e.g. it doesn't really work for > the Scopes.SINGLETON field which is a static final value - which is a > real class loader level immutable singleton. So each Injector stores > the providers returned from the singleton scope instance - not the > singleton Scope; we don't create a singleton Scope object for each > Injector. > Singleton scope gives out self-scoping providers, right? I'm not sure if I see what you're saying here, I should look at your patch first. My rough idea was to delegate to the real scope, and decorate the provider it returns or someting. The other benefit of the patch is we don't introduce another > collection to maintain the singleton objects (as the Injector already > keeps track of the Providers). I don't think that's a real disadvantage. I mean, it's not that we're wasting memory. :-) FWIW the change to the REQUEST/SESSION scopes kinda follow your > approach; though they reuse the existing collections in the scopes > (HttpServletRequest / HttpSession) - so there's no overhead at all > using them; the only new logic is fired if you actually do invoke the > close() method. > > Or to say this another way - I tried my best to support this feature > with the minimal overhead (when being used or not being used) and to > minimise the change to the core of guice. > And I really love the effort. Just trying to see how we could improve it further. > > Whether we use this feature or not on the SINGLETON/REQUEST/SESSION > scopes, we don't introduce any new collections or processing overhead > - unless folks do actually attempt to close resources down. The only > real affect on the core is more internal classes implement the > Closeable interface really - while that might look big in terms of > lines of code changes in a patch, its not actually that big a deal > IMHO. > > > Perhaps I'm just being stupid, because I haven't looked at the patch code, > > really. But I do think it would be nice if we can keep it as an opt-in > > extension, keeping the core API clean. > > The patch doesn't change the public API of Guice at all - other than a > new Injector.close() method. I figured it'd be a more drastic change > to either remove the Scopes.SINGLETON object (as we'd need to create a > Singleton scope object for each Injector) which is part of the public > API and break backwards compatibility - or recommend folks stop using > the scopes in Guice. When you follow best practices and bind to the scope annotation instead of the instance I don't think you'll have a problem with the existing scopes. So you could bind @Singleton to the lifecycle singleton scope, and the rest of your modules don't care. About the close(), It's stuff you need to know, right? It's not that people often use break; and continue; in Java, but they still need to know about it. So adding lifecycle support does impact everyone, even the people who don't use it. People will get confused: "Why is this close() here and when do I call it?" and "Do I need to put that in a finally block to make sure it cleans up?" and so on. I get your point, but it'd be valuable to at least consider making it an extension, instead of dumping it in core Guice without consideration. Anyway, it's not me who you should convince, it's Bob/Jesse. :-) Cheers Robbie --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "google-guice" 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?hl=en -~----------~----~----~----~------~----~------~--~---
