kucci / guava-libraries

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

Multimap.asMap().get() javadoc not consistent with behaviour #437

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Multimap.asMap() javadoc says:
The collections returned by asMap().get(Object) have the same behavior as those 
returned by get(K). Behaviour appears to be defined only when the given object 
is of type K, which is fair.

However, the following shows a discrepancy:
    HashMultimap<String, Integer> hashMultimap = HashMultimap.create();
    hashMultimap.put("a", 2);

    Collection<Integer> fromAsMap = hashMultimap.asMap().get("b"); // null
    Collection<Integer> fromGet = hashMultimap.get("b"); // empty set
    Collection<Integer> fromAsMapWrongType = hashMultimap.asMap().get(new Object()); // null

The problem appears to be in in AbstractMultimap.AsMap#get(Object). We look up 
the key first and return null if there's no current mapping. However this 
appears to be used as an attempt to check the type of the key (declared 
Object). If the key is present, the collection of values is wrapped and 
returned.

Given that we don't know the type of the key at this point, it's difficult to 
see what to do that doesn't involve deferring any ClassCastException to much 
later on. For example the ValuesCollection could store the key as an Object and 
cast on each use, yuk.

==== Test case ====

import static org.junit.Assert.*;

import org.junit.Test;

public class AsMapGetTest {
    @Test
    public void testAsMapGetSameAsGet() throws Exception {
        HashMultimap<String, Integer> hashMultimap = HashMultimap.create();
        hashMultimap.put("a", 2);

        assertEquals(hashMultimap.get("b"), hashMultimap.asMap().get("b")); // spec expects same as get() - fail
    }
}

Original issue reported on code.google.com by joe.j.kearney on 30 Sep 2010 at 3:04

GoogleCodeExporter commented 9 years ago
On second thoughts, trying go get an empty collection from a Map that doesn't 
contain the key seems absurd, and incompatible with the Map spec. I'd vote for 
adding "where the key is associated with any values in the multimap" or 
something to the end of that javadoc and leaving the behaviour as it is.

Original comment by joe.j.kearney on 1 Oct 2010 at 2:49

GoogleCodeExporter commented 9 years ago
This Javadoc statement in accurate, as long as asMap().get() doesn't return 
null.

"The collections returned by asMap().get(Object) have the same behavior as 
those returned by get(K)."

Maybe that sentence should include "if not null". 

Original comment by jl...@google.com on 7 Oct 2010 at 5:55

GoogleCodeExporter commented 9 years ago
That could work. Or "if the key is currently present in the map".

Original comment by joe.j.kearney on 8 Oct 2010 at 7:48

GoogleCodeExporter commented 9 years ago
I clarified the Javadoc.

Original comment by jared.l....@gmail.com on 18 Oct 2010 at 10:28

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 18 Jan 2011 at 8:38

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 24 Jan 2011 at 9:34

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:15

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:09