Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inject the InjectionPoint #27

Closed
gissuebot opened this issue Jul 7, 2014 · 25 comments
Closed

Inject the InjectionPoint #27

gissuebot opened this issue Jul 7, 2014 · 25 comments

Comments

@gissuebot
Copy link

From crazyboblee on February 25, 2007 17:38:40

So they know who they're creating an instance for.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=27

@gissuebot
Copy link
Author

From kevinb9n on February 28, 2007 11:09:37

@Inject Injectee injectee; is really confusing though. :(  Don't know what to do
about this.  I think the name InjectedElement may be better than Injectee, but that
doesn't really solve this problem.

@gissuebot
Copy link
Author

From kevinb9n on March 08, 2007 13:55:34

(No comment was entered for this change.)

Labels: 1.1

@gissuebot
Copy link
Author

From kevinb9n on March 12, 2007 18:11:38

Let's design this thing.  First, I think the name Injectee has just gotta go.

interface InjectionPoint {
  Member getMemberBeingInjected();
  Annotation getBindingAnnotation();
  Set<Annotation> getAllAnnotations();
}

?

For parameter injections, getAllAnnotations() would be a union of those on the
parameter and those on the member.  I think getBindingAnnotation() can be
tremendously useful; in the Module I'd simply bind it by type only, then in my
Provider I can do all manner of different things based on its values.

Of course... I could ask to have the BindingAnnotation injected directly which could
remove the need for me to downcast (I could @Inject WithTimeout timeoutAnnotation)
but it's getting head-trippy.

@gissuebot
Copy link
Author

From dhanji on March 20, 2007 21:15:50

this would be really useful to me, is it in any of the builds/branches so far?

@gissuebot
Copy link
Author

From arikkfir on April 03, 2007 18:16:34

+10!

@gissuebot
Copy link
Author

From kevinb9n on June 03, 2007 11:29:52

Previously I described offering access to:

  * the field, method or constructor being injected
  * the binding annotation instance, if present
  * all of the annotation instances present, including that one

I've now reconsidered the need for this feature (once issue 49 is completed).

I don't think we want to allow injection behavior to be dependent on whether it's a
field or method injection or what the name of the member is or random annotations,
etc.  This makes far too many seemingly-innocuous changes become violent.

What makes sense is to be able to provide differently based on the value of that
binding annotation; without having to bind every single possible value individually
the way you have to today.  And this capability will be provided by Issue 49 .

So at present I am planning to close this issue unless we can hear some really
compelling use cases for things you need to do that this would not enable.

Owner: kevinb9n
Labels: -1.1

@gissuebot
Copy link
Author

From peter.kitt.reilly on June 04, 2007 15:49:15

Would Issue 49 allow binding of Loggers in the
same fashion as is in guice for java.util.logging.Logger ?

@gissuebot
Copy link
Author

From crazyboblee on June 04, 2007 15:52:57

No, in the case of Logger, there's only one actual binding. All you need is the
injectee information.

@gissuebot
Copy link
Author

From kevinb9n on June 04, 2007 16:19:07

The Logger thing works because of internal tricks that Guice does, and this will
continue to be the case.  You won't be able to do the same thing.  It just interacts
way too poorly with scopes.  You can, however, have your binding annotation take a
parameter of type Class, and use the convention

  class MyClass {
    @Inject @MyAnnot(MyClass.class) Foo foo;
  }

@gissuebot
Copy link
Author

From crazyboblee on June 04, 2007 16:23:15

How is the declaring class any different from the binding annotation's attributes
when it comes to reacting badly with scopes?

@gissuebot
Copy link
Author

From kevinb9n on June 04, 2007 16:32:56

This discussion should be happening over in issue 49 , not here (this is the issue I
want to close as won't fix).

The binding annotation's attributes become part of the Key, and so they get scoped
properly.

@gissuebot
Copy link
Author

From crazyboblee on June 04, 2007 16:37:54

OK. I still need to catch up.

@gissuebot
Copy link
Author

From peter.kitt.reilly on June 05, 2007 02:48:11

#9 is nearly as bad as the boilerplate it is replacing.
Eversince I hace seen log4j and other logging packages one
of the three things that have annoyed me is the need for
redeclaring the class in the incantation of declaring the
logger. The second thing that annoyed me was the forced use
of the factory pattern. The third thing was the need for if enabled
wrappers around the log statement.

Seam provides a Logger that solves all three (at the cost of using
seam ;-). Guice provides a Logger that solves one and two (at the cost
of using java.util.logging). #9 provides a solution for two and three
but not one.

@gissuebot
Copy link
Author

From bslesinsky on July 07, 2007 16:57:43

I'm somewhat concerned that giving the provider access to the
java.lang.reflect.Member for an injectee would allow providers to add non-explicit
behavior in such a way that the injectee is not free to refactor.  (For example,
doing something different depending on the name of the method, whether it's
constructor/method/field injection, or which class or package the injection is
happening in.)  Are there use cases for that, or would providing the annotations suffice?

It seems like having a clear rule that injection is based solely on annotations and
the bound type would be less likely to result in surprises.

@gissuebot
Copy link
Author

From jdpatterson on October 09, 2007 08:15:24

I agree with the above comment, that the Key used to select the provider should be sufficient for most purposes.

@gissuebot
Copy link
Author

From dh.evolutionnext on May 20, 2008 10:34:04

I disagree with that this has anything to do directly with Issue 49 .   I also
disagree with 14, 15.

My use case is that I need to create a Scope based on what Swing Component is
requiring the injection.  In detail, what I need is a JInternalFrame Scope.
Therefore this is of great interest to me.

I do like what Issue 49 is getting at, if I read and understood right.  I really
think it will put Guice over the top as the premier DI framework if we can inject
based on what object, method, package, closure*, block*, thread, or stage is
requiring the injected object.  It would really be powerful if we can also get an
injection based on regular expression too.

(* = not required now, but looks though as if things like closures and various other
blocks will be included in future versions of java)

As per the name "injectee", other names that could be used "source", "destination",
or "requestor".   My vote is on "source" since it is consistent with the java's
listeners which typically offers a getSource() method.

@gissuebot
Copy link
Author

From limpbizkit on May 30, 2008 00:18:41

Deprioritizing. This is an interesting feature but it seems to encourage fragile code. It's rare that a method needs
to know the calling method, and I believe this is similar.

Summary: Provide "injectee" to custom providers.
Status: New
Labels: -Priority-Medium Priority-Low

@gissuebot
Copy link
Author

From limpbizkit on May 30, 2008 00:24:39

(No comment was entered for this change.)

Summary: Inject the InjectionPoint

@gissuebot
Copy link
Author

From kobevaliant on November 24, 2008 01:59:13

This is cool!
Runtime injection can be implemented by this feature plus custom provider

@gissuebot
Copy link
Author

From jose.illescas on February 09, 2009 10:48:50

See patch on https://code.google.com/p/google-guice/issues/detail?id=258 note: runtime injection across custom provider

@gissuebot
Copy link
Author

From sberlin on April 25, 2010 15:06:05

Issue 440 has been merged into this issue.

@gissuebot
Copy link
Author

From kevinb9n on April 25, 2010 15:09:55

I continue to believe this feature would be a bad idea.  It's simply backwards.  It will become even harder to
reason about your application when things like this can happen.

@gissuebot
Copy link
Author

From ogregoire on April 26, 2010 02:28:06

I understand people may have some reluctance, but I think that Guice has clearly
taken the path of allowing 3rd party providers to enhance what Guice offers. I do
really think this feature would be a great step forward in that way. And since it
would be purely SPI, I ought to think that people would really know what they are
doing with this.

This functionality would clearly be designed for SPI, so used by people who are
supposed to know what they do.

@gissuebot
Copy link
Author

From sberlin on May 02, 2010 05:32:28

Issue 412 has been merged into this issue.

@gissuebot
Copy link
Author

From sberlin on February 19, 2011 12:23:12

It's been 4 years since the issue was opened, Guice has seen wide use, and the general consensus thus far has been to purposely not expose this feature.  It makes debugging even harder than it already is (which is the #1 pain point with Guice).

Status: WontFix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant