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

Multibindings #37

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

Comments

Projects
None yet
1 participant
@gissuebot
Copy link

commented Jul 7, 2014

From kevinb9n on February 27, 2007 03:21:14

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: http://code.google.com/p/google-guice/issues/detail?id=37

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From kevinb9n on February 27, 2007 00:22:04

(No comment was entered for this change.)

Summary: Multibindings

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From limpbizkit on March 02, 2007 09:23:37

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From kevinb9n on March 08, 2007 14:13:47

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

Labels: 1.1

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From smiddlek on March 17, 2007 12:09:51

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From crazyboblee on March 17, 2007 12:24:41

A slightly shorter syntax:

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

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From crazyboblee on March 17, 2007 12:35:07

A more descriptive syntax:

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From crazyboblee on March 17, 2007 12:44:08

Yet another:

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From lwerner on March 17, 2007 17:54:07

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From kevinb9n on March 18, 2007 00:08:04

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From dhanji on March 18, 2007 16:09:02

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From kevinb9n on April 16, 2007 10:04:00

Dhanji, I believe my proposal above does address this.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From bslesinsky on May 17, 2007 11:19:01

Another possible syntax:

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From bslesinsky on July 25, 2007 09:57:32

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From robbie.vanbrabant on August 08, 2007 03:36:40

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From robbie.vanbrabant on August 13, 2007 04:43:58

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));

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From dhanji on September 01, 2007 06:18:57

Note that "for" is a reserved word in Java =(

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From robbie.vanbrabant on October 11, 2007 07:55:35

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From logan.johnson on December 12, 2007 06:27:39

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From bslesinsky on December 12, 2007 08:38:54

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From logan.johnson on December 12, 2007 08:48:20

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.

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From logan.johnson on March 31, 2008 20:25:17

Related discussion on the user mailing list: http://groups.google.com/group/google-guice/t/640b245f49a2b802

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From limpbizkit on April 29, 2008 11:07:39

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

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From earwin on April 29, 2008 13:27:41

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?

@gissuebot

This comment has been minimized.

Copy link
Author

commented Jul 7, 2014

From limpbizkit on May 01, 2008 03:12:16

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.

Status: Fixed
Owner: limpbizkit

@gissuebot gissuebot closed this Jul 7, 2014

leusonmario pushed a commit to leusonmario/guice that referenced this issue Aug 16, 2018

Merge pull request google#37 from littson/master
Added mybatis.configuration.defaultStatementTimeout inject property in ConfigurationProvider.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.