xiaodududu / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Multibindings #37

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
It would be good if certain keys could be multivalued; where multiple
modules could append one or more bindings each, and these could all be
injected as a Collection<Foo> or something.  This would be useful for some
things Laura is doing, and for replacing excalibur, and for
AdWordsPaymentOptions, and . . .

I'm going ahead and calling this priority "high" for now.

Original issue reported on code.google.com by kevin...@gmail.com on 27 Feb 2007 at 8:21

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 27 Feb 2007 at 8:22

GoogleCodeExporter commented 9 years ago
A rough sketch of possible syntax:

bind(Number.class).asDefaultCollectionElement().to(new Integer(5));
bind(Number.class).asCollectionElement().to(new Integer(7));
bind(Number.class).asCollectionElement().to(new Long(13));

...

public @Inject Number foo; // injects 5
public @Inject List<Number> bar; // injects List 5, 7, 13
public @Inject Collection<Number> baz; // injects List 5, 7, 13

- It might be better without the  asDefaultCollectionElement() method, and to 
require
5 to be bound twice. That way you don't have to worry about binding 5 alone to
Integer and 5 as a collection to List<Number>.
- We don't know the unerased runtime types of bar and baz, so this might require
annotations or something else fancy.
- Perhaps we'd be better off recommending the user create a holder for their
collections to avoid the erasure issues:
  class MyFavouritePrimes {
    @Inject List<Number> values; // since there's no List injected, Guice could
inject all Numbers, regardless of their annotation/key?
  }

- Just as we forbid injecting Guice classes, why not forbid anything with a 
classname
in java.* that isn't qualified with an annotation? Ie. it would be okay to do 
this:
   @Inject @Blue Color color
... but not this:
   @Inject Color color

Original comment by limpbizkit on 2 Mar 2007 at 5:23

GoogleCodeExporter commented 9 years ago
(to the last part: I was thinking about exactly that a couple days ago!  It 
seems a
little less than fully generalized, but like a basically good idea.

Original comment by kevin...@gmail.com on 8 Mar 2007 at 10:13

GoogleCodeExporter commented 9 years ago
I'm glad to see this is being addressed in 1.1!  I think it would be important 
to
allow for different collections of the same type (MyFavoritePrimes and
MyFavoriteFibonacciSeq), possibly through annotations.

bind(Number.class).asCollectionElement().annotatedWith(...)

I'd also love to see support for Maps. :)  I believe Kevin saw some hacked 
together
code to get this done in the UnityTaskServer when he migrated our code to Guice 
1.0.

Original comment by smidd...@gmail.com on 17 Mar 2007 at 7:09

GoogleCodeExporter commented 9 years ago
A slightly shorter syntax:

bind(Foo.class).inCollection().annotatedWith(...).to(FooImpl.class).in(Scopes.SI
NGLETON);

It's interesting to note that each element can have a different scope.

Original comment by crazybob...@gmail.com on 17 Mar 2007 at 7:24

GoogleCodeExporter commented 9 years ago
A more descriptive syntax:

bindElementInCollectionOf(Foo.class).annotatedWith(...).to(FooImpl.class);

Original comment by crazybob...@gmail.com on 17 Mar 2007 at 7:35

GoogleCodeExporter commented 9 years ago
Yet another:

multibind(Foo.class).annotatedWith(...)...

Original comment by crazybob...@gmail.com on 17 Mar 2007 at 7:44

GoogleCodeExporter commented 9 years ago
Since it looks like I was one of the original requestors of this... :-)

One thing I've discovered while using my current workaround for this missing 
feature
(injecting a Collection<Foo> into a static method which adds elements to the
collection) is that in some cases you want to be able to specify the order of 
the
elements in the injected collection.  So if the things in the collection are 
menu
items contributed by an application's plugins, I want to be able to specify some
ordering for the items.

I'm not 100% sure that this is Guice's problem.  It's almost as easy for 
clients to
copy the injected collection and then sort it if they need to.  But having Guice
cache the sorted version would be nice too.

One idea I'm playing with is to have my collection notice whether the items you 
put
into it implement Comparable, and if they do then sort them.  I don't know if 
it's
going to work well yet, since I'm just playing with this in my "spare" time, 
but it
seems promising.

All of this is pretty speculative, but I figured I should get it written down
somewhere.  Bob or Kevin, feel free to come talk to me if you want to discuss 
this or
see some code that could use it.

Original comment by lwer...@gmail.com on 18 Mar 2007 at 12:54

GoogleCodeExporter commented 9 years ago
For the modules which are contributing individual bindings to the collection, I 
kind
of like the syntax

  bind(Handler.class).XXX().to(MyBlueHandler.class);
and
  bind(Handler.class).annotatedWith(Green.class).XXX().to(MyGreenHandler.class);

because this is only one new method we have to add, not a new method like 
multibind()
which we'd need three overloads of (Class, TypeLiteral, Key).

Only problem is I cannot think of what name for XXX() doesn't suck awfully.

This still leaves undefined what kind of Collection we should instantiate as 
well as
what type we should allow you to @Inject.  Do we create a HashSet and you 
@Inject a
Collection?  Or do we create an ArrayList and you @Inject an Iterable?  Etc.

Rather than deciding that in the framework I think we can just have the module 
that
wishes to aggregate these bindings define the binding and provider that we'll 
use:

  bind(new TypeLiteral<Set<Handler>>() {})
      .toProvider(new Provider<Set<Handler>>() {
          public Set<Handler> get() {
            return new HashSet<Handler>();
          });

If we see this, we will then support "@Inject Set<Handler>" but not "@Inject
Collection<Handler>" or anything else; furthermore, we'll know how to create new
Collection instances; the user can specify SortedSet or ConcurrentSkipListSet or
whatever.

In fact, the user can even apply a scope to the collection binding; we'll 
simply have
to validate that if any element bindings are also scoped, they must be at the 
same
scope as the collection, or a superscope.  Did I get that right?  It is almost 
as if
the Collection binding contains an @Inject request for each of the specific 
element
bindings (although it doesn't have anything of the sort, really).

Now what's missing is that guice really should inject only an unmodifiable view 
of
the collection.  It's a bit of a pain to make something unmodifiable in a 
general way
without knowing specifically that you're dealing with a SortedSet or something, 
but
we can hack together the code to do it.

Original comment by kevin...@gmail.com on 18 Mar 2007 at 7:08

GoogleCodeExporter commented 9 years ago
I like this idea very much but we need to have a way to specify the collection
semantic. contributing duplicates should be an option:

bind(Number.class).collectAs(List.class).to(new Long(5));
bind(Number.class).collectAs(Set.class).to(new Long(5));

..should have very different meanings (I may want to contribute 5, 5, 5 as a 
list for
instance).

Original comment by dha...@gmail.com on 18 Mar 2007 at 11:09

GoogleCodeExporter commented 9 years ago
Dhanji, I believe my proposal above does address this.

Original comment by kevin...@gmail.com on 16 Apr 2007 at 5:04

GoogleCodeExporter commented 9 years ago
Another possible syntax:

allowMultiple(Foo.class);
bindCollection(MyFooList.class).toAll(Foo.class);
bind(Foo.class).to(MyFoo.class);
bind(Foo.class).to(AnotherFoo.class);

Original comment by bslesinsky on 17 May 2007 at 6:19

GoogleCodeExporter commented 9 years ago
This functionality seems like a static version of the whiteboard pattern from 
OSGi;
we should probably see what we can learn from them.

Original comment by bslesinsky on 25 Jul 2007 at 4:57

GoogleCodeExporter commented 9 years ago
http://www.osgi.org/documents/osgi_technology/whiteboard.pdf

Original comment by robbie.v...@gmail.com on 8 Aug 2007 at 9:50

GoogleCodeExporter commented 9 years ago
Another idea:
bind(new TypeLiteral<List<SomeClass>>(){}).toProvider(
    MultiBindProvider.for(new TypeLiteral<List<SomeClass>>(){}, ArrayList.class)
                     .withElements(Fast.class, Slow.class)
);
bind(SomeClass.class).annotatedWith(Fast.class).toInstance(new SomeClass(true));
bind(SomeClass.class).annotatedWith(Slow.class).toInstance(new 
SomeClass(false));

Perhaps not the most elegant solution, but straightforward and backwards 
compatible.
Not sure about Maps though.

Original comment by robbie.v...@gmail.com on 8 Aug 2007 at 10:36

GoogleCodeExporter commented 9 years ago
Same thing, but with issue 123 syntax:
bind(listOf(SomeClass.class)).toProvider(
    MultiBindProvider.for(listOf(SomeClass.class), ArrayList.class)
                     .withElements(Fast.class, Slow.class)
);
bind(SomeClass.class).annotatedWith(Fast.class).toInstance(new SomeClass(true));
bind(SomeClass.class).annotatedWith(Slow.class).toInstance(new 
SomeClass(false));

Original comment by robbie.v...@gmail.com on 13 Aug 2007 at 11:43

GoogleCodeExporter commented 9 years ago
Note that "for" is a reserved word in Java =(

Also it should be possible to specify a key for the collection implementation.

Original comment by dha...@gmail.com on 1 Sep 2007 at 1:18

GoogleCodeExporter commented 9 years ago
Another idea; use a special Module, much like AbstractModule:

install(new AbstractMultibindModule() {
  public void configure() {
    // do the usual stuff
  }
  // add optionally overridable getKey() method
  // that specifies the target collection explicitely
});

By using that module, Guice could tag all bindings that happen in that module 
for
multibind. This avoids some of the repetition you'd have with XXX() and keeps 
the
binder syntax clean.

Original comment by robbie.v...@gmail.com on 11 Oct 2007 at 2:55

GoogleCodeExporter commented 9 years ago
I know this conversation is old, and I hope I haven't missed the boat entirely. 
 The
main use of multibindings that I can see is registering plugins, listeners, and 
the
like.  Why not do something like this?:

class SomeEventSource implements EventSource {
   @Inject(optional=true,multiple=true)
   public void addListener(Listener l);
   }
}

class SomeModule extends AbstractModule {
  inject(SomeListener.class).into(EventSource.class).as(Listener.class);
  inject(SomeOtherListener.class).into(EventSource.class).as(Listener.class);
}

Controlling order is up to the user.  You could define a priority scheme in your
Listener interface, or do simple sorting in your EventSource, or whatever.

Original comment by logan.jo...@gmail.com on 12 Dec 2007 at 2:27

GoogleCodeExporter commented 9 years ago
Interesting.  I think we could just use regular binder syntax; the only thing 
needed
is a way to declare that a key allows multiple bindings.  Binding a 
multiple-bound
key to a single-bound injection point would just be an error.

I could see people still wanting arrays, lists, or sets of values to be 
injected,
particularly since calling the same method multiple times only works for
single-argument method injection.  But "adder injection" (to invent a name for 
it)
could be implemented first and the rest added later if needed.

Original comment by bslesinsky on 12 Dec 2007 at 4:38

GoogleCodeExporter commented 9 years ago
If we had "adder injections" (good name), then injecting collections/iterables 
would
just be one level of indirection away with no changes to Guice:

class SortedPlugins implements Provider<List>, PluginRegistry {
  @Inject(multiple=true)
  public void registerPlugin(Plugin p) { ... }

  public List<Plugin> get() { ... }
}

The biggest difference between my suggestion and the ones I've seen before is 
that
mine specifies the injection target.  This avoids some ambiguity of intent with
multibindings.

Original comment by logan.jo...@gmail.com on 12 Dec 2007 at 4:48

GoogleCodeExporter commented 9 years ago
Related discussion on the user mailing list:

http://groups.google.com/group/google-guice/t/640b245f49a2b802

Original comment by logan.jo...@gmail.com on 1 Apr 2008 at 3:25

GoogleCodeExporter commented 9 years ago
I blogged about my API proposal, which doesn't require changes to core Guice:
http://publicobject.com/2008/04/guice-multibinder-api-proposal.html

Original comment by limpbizkit on 29 Apr 2008 at 6:07

GoogleCodeExporter commented 9 years ago
I used an alternative, no Guice alterations either, shorter syntax than your 
version,
but limited in some cases. Posted a comment in your blog, but it's not yet 
visible.
Should I repost here?

Original comment by earwin@gmail.com on 29 Apr 2008 at 8:27

GoogleCodeExporter commented 9 years ago
Initial draft implemented. Announcement here:
http://groups.google.com/group/google-guice/browse_frm/thread/d601e981f5a90e4

Still unsupported:
 - anything other than Set
 - ordering

I'm going to close this issue and see if that's sufficient. If anyone wants 
more features, open new bugs! I 
suspect this is 90% of what we need, and the last leg can be accomplished 
within the user code.

Original comment by limpbizkit on 1 May 2008 at 10:12