On 1 June 2010 11:12, Pascal-Louis <[email protected]> wrote:

> Hi all,
>
> We've had a long standing grudge against Guice (which we love in all
> other respects!): the inability to have user-defined just-in-time
> bindings.
>
> Over the course of the long weekend, I've implemented this feature and
> would love some feedback. Check it out on
> http://eng.kaching.com/2010/05/just-in-time-providers-for-guice.html.
>
> I would specifically appreciate feedback from the Guice team on the
> process to integrate this patch into trunk.


[caveat: just my personal opinion as a fellow patcher...]

people usually open issues for feature requests / patches:

   http://code.google.com/p/google-guice/issues/list

as its much easier to track and review than email threads

I'd also suggest cleaning up the patch so it just contains
the absolute necessary changes for the new feature - the
current patch includes a lot of non-functional / formatting
changes which makes it hard to read and digest

On a more tactical note, I
> was unsure about which injection points to use, see the ??? comment in
> the method
>
> http://github.com/pascallouisperez/guice-jit-providers/blob/master/src/com/google/inject/internal/InjectorImpl.java#LID830
> .
>

for ProviderInstanceBindingImpl it should be the injection
points of the provider instance - in the patch this is just a
wrapper around "jitProvider" so it has no injection points,
which means you could simply use:

   ImmutableSet.<InjectionPoint>of()

this should also remove the need to change InjectionPoint
to handle null superclasses - the InjectionPoint SPI should
only be used on classes (not interfaces) so it always hits
the Object check first - when you used the request key it
might be an interface, which I guess is why you added the
null check to InjectionPoint

btw, are you arranging for the "jitProviders" to be injected
so that they can also contain @Inject dependencies?

Best,
> PL
>
> --
> 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]<google-guice-dev%[email protected]>
> .
> For more options, visit this group at
> http://groups.google.com/group/google-guice-dev?hl=en.
>

-- 
Cheers, Stuart

-- 
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.

Reply via email to