Lack of documentation in Multimap.putAll(K key, Iterable<? extends V> values) ?

377 views
Skip to first unread message

Sylvain MINA

unread,
Jun 22, 2010, 8:32:26 AM6/22/10
to guava-...@googlegroups.com
Hi all, 

Since I use google-collections and now guava, I wanted to use Multimap in a wrong way.
I replace Map<K, Collection<V>> by Mulitmap<K,V>, but as a result some of UT were broken. Indeed, I wanted use the putAll(K, Iterable<V>) with an empty Iterable to "initialize" the multimap content. I wondered why it didn't work... I saw nothing about that in the javadoc, but I understood the behavior when I read the sources.

Am I the only one who has been tricked by this ? Would it be interresting to mention the behavior in the javadoc ? Is it a bad smell if I want to populate a multimap in this way ?

(Perhaps this question have been already asked, but I didn't find it, neither in the mailing list, nor in the issues, nor on stackoverflow.com)

(Apologize for my frenglish....)


Dimitris Andreou

unread,
Jun 22, 2010, 9:06:27 AM6/22/10
to guava-...@googlegroups.com
Basically the central feature that makes Multimap<K, V> more convenient that Map<K, Collection<V>> - you don't have to mess with manually adding and removing those collections. If this behavior doesn't fit your case, then it is not a good match.

If you ask if you are the only one tricked, then certainly not - at least I was. :) A bit more indirectly though, but based on this behavior: I was iterating over the keys of a multimap, accessing the collection of each key, adding/removing elements from that collection. But if the collection went empty - boom! ConcurrentModificationException in the key iteration, because when the value collection got empty the key is automatically removed. 

As per the javadocs, Multimap is "A collection similar to a Map, but which may associate multiple values with a single key." Perhaps that "multiple" should be clarified a bit, because it also means 1 value per key, but it doesn't mean 0 values per key. 

Dimitris

2010/6/22 Sylvain MINA <sylvai...@gmail.com>
--
guava-...@googlegroups.com.
http://groups.google.com/group/guava-discuss?hl=en
unsubscribe: guava-discus...@googlegroups.com
 
This list is for discussion; for help, post to Stack Overflow instead:
http://stackoverflow.com/questions/ask
Use the tag "guava".

Jared Levy

unread,
Jun 22, 2010, 11:20:30 AM6/22/10
to guava-...@googlegroups.com
The Multimap.putAll() Javadoc is vague. We'll fix it; thanks for pointing out the issue.

Multimap.replaceValues() provides the functionality you're looking for.


sylvain.mina

unread,
Jun 23, 2010, 8:22:08 AM6/23/10
to guava-discuss
I did'nt see that I were replying only to Dimitris... so

On Tue, Jun 22, 2010 at 5:32 PM, Dimitris Andreou
<jim.a...@gmail.com> wrote:
By the way, sorry for misunderstanding your question, got entirely the
wrong idea :) I only realized seeing Jared's reply.


I don't know if Jared replies to you or me, but in my case, the
Multimap.replaceValues() does'nt work as I expect.
Indeed, when I replace

final Map<Key, Iterable<Value>> map = Maps.newHashMap();
for (final Key key : keys) {
map.put(process, getPerhapsValues());
}

by

final Multimap<Key, Value> map = ArrayListMultimap.create();
for (final Key process : processes) {
map.replaceValues(process, getPerhapsValues());
}

if, the function getPerhapsValues() returns an empty iterable, then in
the Map I have some entries like <key, [empty]>, but in the second
one, these entries don't exist in the Multimap.

Anyway, I got my answer, and the javadoc will be updated to be more
explicit, Thanks a lot :)

On Jun 22, 5:20 pm, Jared Levy <jared.l.l...@gmail.com> wrote:
> The Multimap.putAll() Javadoc is vague. We'll fix it; thanks for pointing
> out the issue.
>
> Multimap.replaceValues() provides the functionality you're looking for.
>
> On Tue, Jun 22, 2010 at 5:32 AM, Sylvain MINA <sylvain.m...@gmail.com>wrote:
>
>
>
> > Hi all,
>
> > Since I use google-collections and now guava, I wanted to use Multimap in a
> > wrong way.
> > I replace Map<K, Collection<V>> by Mulitmap<K,V>, but as a result some of
> > UT were broken. Indeed, I wanted use the putAll(K, Iterable<V>) with an
> > empty Iterable to "initialize" the multimap content. I wondered why it
> > didn't work... I saw nothing about that in the javadoc, but I understood the
> > behavior when I read the sources.
>
> > Am I the only one who has been tricked by this ? Would it be interresting
> > to mention the behavior in the javadoc ? Is it a bad smell if I want to
> > populate a multimap in this way ?
>
> >  (Perhaps this question have been already asked, but I didn't find it,
> > neither in the mailing list, nor in the issues, nor on stackoverflow.com)
>
> > (Apologize for my frenglish....)
>
> >  --
> > guava-...@googlegroups.com.
> >http://groups.google.com/group/guava-discuss?hl=en
> > unsubscribe: guava-discus...@googlegroups.com<guava-discuss%2Bunsubscribe@goog legroups.com>

Kevin Bourrillion

unread,
Jun 23, 2010, 10:49:06 AM6/23/10
to guava-...@googlegroups.com
A multimap is a collection of key-value pairs, exactly like a map (just without the duplicate-key restriction).  If a map contained zero mappings from the key k, you wouldn't expect k to appear in its keySet, and multimap is just the same.  It sounds like what you really want here is a plain old Map<K, Collection<V>>.  Or use the Multimap and keep a separate Set<K> for all the keys you care about.


--
Kevin Bourrillion @ Google
http://guava-libraries.googlecode.com

Sylvain MINA

unread,
Jun 23, 2010, 10:58:51 AM6/23/10
to guava-...@googlegroups.com
A multimap is a collection of key-value pairs, exactly like a map (just without the duplicate-key restriction).  If a map contained zero mappings from the key k, you wouldn't expect k to appear in its keySet, and multimap is just the same.  It sounds like what you really want here is a plain old Map<K, Collection<V>>.  Or use the Multimap and keep a separate Set<K> for all the keys you care about.

Thanks for the explanation, it's exactly what I understood when I read the code. I agree with this, but I just want to point out the fact that this behavior was not explicit to me we I read the javadoc.
Reply all
Reply to author
Forward
0 new messages