change Multimap.asMap to return Map<K,? extends Collection<V>>

[GoogleCodeExporter]

What steps will reproduce the problem?
N/A

What is the expected output? What do you see instead?
N/A

What version of the product are you using? On what operating system?
0.5 (ALPHA) 

Please provide any additional information below.
The Multimap.asMap method currently returns Map<K,Collection<V>>, which
restricts the return types of the ListMultimap and SetMultimap.  By
changing the return type to Map<K,? extends Collection<V>>, you would allow
the sub-interfaces to return more appropriate types (i.e. ListMultimap
could return Map<K,? extends List<V>>).

Original issue reported on code.google.com by jahlborn@gmail.com on 22 Oct 2007 at 2:57

[GoogleCodeExporter]

Thanks for the suggestion. It would be nice if that was feasible. 
Unfortunately, when
I tried implementing that method signature, it made asMap() more difficult to 
use.

The main usage of asMap() is when people iterate across 
multimap.asMap().entrySet().
When asMap returned Map<K, ? extends Collection<V>>, the application code 
reported
confusing "capture of ?" compilation errors. The caller could work around that 
with
some ugly casting, but leaving the code as-is is cleaner.

Another possibility just occurred to me. We could add the following method to
ListMultimap (and similar methods to SetMultimap and SortedSetMultimap):
  Map<K, List<V>> asListMap()

That provides the functionality you want -- avoiding casts in the application 
code --
without the messiness of generic wildcards.

Original comment by jared.l....@gmail.com on 22 Oct 2007 at 5:12

[GoogleCodeExporter]

Original comment by jared.l....@gmail.com on 22 Oct 2007 at 5:12

[GoogleCodeExporter]

asListMap() feels like such a hack (but might be better than nothing).

It's very sad that this change really seemed to overwhelm javac's ability to 
make
sense of the generics.  Jared, can you give us an example of what some typical 
client
code would have to look like (before vs. after) if this change were made?

Original comment by kevin...@gmail.com on 22 Oct 2007 at 5:44

[GoogleCodeExporter]

Here's a typical usage of asMap():

  public static void printValueSizes(Multimap<String, Integer> multimap) {
    Map<String, Collection<Integer>> map = multimap.asMap();
    Set<Map.Entry<String, Collection<Integer>>> es = map.entrySet();
    for (Map.Entry<String, Collection<Integer>> entry : es) {
      String key = entry.getKey();
      Collection<Integer> values = entry.getValue();
      System.out.println(key + " has " + values.size() + " values");
    }
  }

If we change the Multimap.asMap() signature to
  Map<K, ? extends Collection<V>> asMap();

I couldn't find a clean way to adjust that iteration logic. Here's the best I 
could
come up with:

  public static void printValueSizes(Multimap<String, Integer> multimap) {
    Map<String, ? extends Collection<Integer>> map = multimap.asMap();
    Set<?> es = map.entrySet();
    for (Object object : es) {
      @SuppressWarnings("unchecked")
      Map.Entry<String, ? extends Collection<Integer>> entry =
        (Map.Entry<String, ? extends Collection<Integer>>) object;
      String key = entry.getKey()
      Collection<Integer> values = entry.getValue();
      System.out.println(key + " has " + values.size() + " values");
    }
  }

That's really ugly, but nothing else compiles. Unless a generics guru has a 
better
idea, asListMap() is the best we can do.

Original comment by jared.l....@gmail.com on 1 Nov 2007 at 5:43

[GoogleCodeExporter]

what am i missing?  i'm not using multimap, just a simple map, but this code 
compiles
and runs fine on my box (jdk 1.5.0_12-b04):

[code]
  public static void main(String[] args) throws Exception
  {
    Map<String, List<Integer>> tmp =
      new HashMap<String, List<Integer>>();
    tmp.put("foo", Arrays.asList(42));

    Map<String, ? extends Collection<Integer>> map = tmp;
    for(Map.Entry<String, ? extends Collection<Integer>> e : map.entrySet()) {
      System.out.println("Value " + e.getValue());
    }

  }
[/code]

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 1:56

[GoogleCodeExporter]

jahlborn, the issue here has to do with Multimap.  I don't see the connection 
between
the issue and this code you pasted.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 4:18

[GoogleCodeExporter]

jahlborn's example is relevant. The iterator does work, though you can't define 
the
entry set variable as an intermediate step.

  public static void printValueSizes(Multimap2<String, Integer> multimap) {
    Map<String, ? extends Collection<Integer>> map = multimap.asMap();
// WON'T COMPILE:   Set<Map.Entry<String, ? extends Collection<Integer>>> es =
map.entrySet();
    for (Map.Entry<String, ? extends Collection<Integer>> entry : map.entrySet()) {
      String key = entry.getKey();
      Collection<Integer> values = entry.getValue();
      System.out.println(key + " has " + values.size() + " values");
    }
  }

We have the option of changing the method signature and iterating across the 
entry
set, while being unable to store the entry set in a separate variable. I'm not
convinced that's an improvement, but it's plausible.

Original comment by jared.l....@gmail.com on 1 Nov 2007 at 4:36

[GoogleCodeExporter]

Ok, it's probably just too early in the morning for me.

My main concern is that we don't make regular usage patterns by regular folks in
regular situations more confusing/ugly/bulky.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 4:55

[GoogleCodeExporter]

That's why I'm leaning toward adding methods like ListMultimap.asListMap(). In 
my
experience, methods that return generics with "?" are difficult to use.

Original comment by jared.l....@gmail.com on 1 Nov 2007 at 6:30

[GoogleCodeExporter]

It is also conceivable that we just do nothing and clients have to cast.

Clearly our library wants you to never have to cast, and that's good.  But 
there may
be a level of diminishing returns at some point in that effort.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 7:19

[GoogleCodeExporter]

Ah, now i see your concern.  In order to hold the intermediate set, you need:

Set<? extends Map.Entry<String, ? extends Collection<Integer>>> eSet = 
map.entrySet();

which is a bit on the ugly side.  however, to kevin's point about making this 
code
useable, i would argue most people do not hold the intermediate entrySet, but 
iterate
over it as in my example.  and, if they do that, then they do not need to case 
the
e.getValue() result which is *very* nice.

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 7:39

[GoogleCodeExporter]

oops.  s/case/cast/

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 7:40

[GoogleCodeExporter]

FYI, another similar issue to keep an eye on:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6231312

Original comment by kevin...@gmail.com on 1 Nov 2007 at 9:56

[GoogleCodeExporter]

After considering the alternatives, I've concluded that maintaining the status 
quo is
the best choice. Adding "? extends" to the interface makes it more difficult to 
use.
A manual cast seems better than creating methods like asListMap().

Original comment by jared.l....@gmail.com on 3 Nov 2007 at 12:24

• Changed state: WontFix