commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dipanjan Laha (JIRA)" <>
Subject [jira] [Commented] (COLLECTIONS-508) MultiMap's methods are not strongly typed even though the interface supports generics
Date Tue, 04 Mar 2014 18:59:20 GMT


Dipanjan Laha commented on COLLECTIONS-508:

Hi Thomas,

Thanks for looking into the patch. Apologies for the duplicate files, must have messed up
somewhere while created the patch :)
Please find my comments below.

> * Set<Entry<K, Collection<V>>> entrySet():
>   I am not so sure if this really makes sense, I think it would be better to
>  have a method like Collection<Entry<K, V> entries
Yes, this definitely makes better sense. I will change the implementation appropriately .

> * boolean containsValue(Object key, Object value): to be consistent with
> removeMapping, I would propose to name it similar, either both
>   xxxMapping or xxxEntry, tbd
IMHO I think containsMapping goes well with removeMapping. What do you think?

> * it would also be interesting to have a Bag<K> keys() method that 
> returns a view of all keys and how many mappings there are for each, 
> but see also below
Find my comments below

> * size && totalSize: I would prefer that size returns the total number of 
> mappings (what totalSize does now).  The number of keys can be 
> retrieved with keySet().size() imho.
I couldn't agree more. I kept it so that it would be consistent with the MultiValueMap implementation,
but it is quite confusing. I will make the proposed changes.

> * a putAll(K key, Iterable<? extends V> values) would be nice too
I will add this method too.

> Something we have not considered yet is the semantic of the actual
> Collection type used in the MultiValuedMap. It might have a List or Set 
> behavior, i.e. allowing duplicates or not. We could make specific 
> interfaces for the different types of MultiValuedMaps derived from 
> MultiValuedMap.
We can surely have specific interfaces for different types of MultiValuedMaps. I was thinking
on the lines that we can have a flag like allowDuplicates in the implementing class which
can be taken as an argument in a constructor. But your proposal is better as we can then have
strongly typed methods which can return a List or a Set depending upon the behavior. My only
concern is that we would end up with two implementations for each type of MultiValuedMap,
one each for Set & List. If this is fine then we can go ahead with this. One more thing,
then should we have MultiValuedHashMap anymore? It would be MultiValuedSetHashMap and MultiValuedListHashMap
which can implement MultiValuedMap(common to both) and MultiValuedSetMap and MultiValuedListMap
individually(In this case we also need to think about the names too). What do you think?

>Regarding the Bag issue: actually I wanted to clean up the Bag interface
> to be fully Collection compliant for 4.0 but we decided to keep it as is. As 
> a workaround there is now a CollectionBag decorator to make a Bag 
> compliant to the Collection contract. We could also add now a new 
> MultiValuedSet interface that is basically the same as a CollectionBag.
>What do you think?
I think the Bag<K> keys() method can be really useful IMHO. We can have a new MultiValuedSet
interface, but that would be kind of moving away from the Bag interface. Wouldn't it be better
to clean up the Bag interface or would it hamper the backward compatibility? In that case
we can have a MultiValuedSet interface and have a MultiValuedSetWrapper(or some other name)
class for wrapping the existing Bag implementations or have a completely different set of
implementations for MultiValuedSet. I think IMHO that we should hold on with the Bag<K>
keys() method till we decide on the above issue. Let me know your thoughts.

> MultiMap's methods are not strongly typed even though the interface supports generics
> -------------------------------------------------------------------------------------
>                 Key: COLLECTIONS-508
>                 URL:
>             Project: Commons Collections
>          Issue Type: Improvement
>          Components: Map
>    Affects Versions: 4.0
>            Reporter: Dipanjan Laha
>         Attachments: MultiValuedMap.patch
> Recently I had the need of using a MultiMap in one of my projects. While using the same,
I found that the MultiMap interface  has methods that are not strongly typed even though the
interface supports generics. For example if I have a MultiMap like so
> MultiMap<String, User> multiMap = new MultiValueMap<String, User>();
> where User is a custom  Class, then the get(key) method would return me an Object which
I would need to cast to a Collection like so
> Collection<User> userCol = (Collection<User>) multiMap.get(key);
> I understand that this limitation comes from that fact that the MultiMap extends IterableMap
which in turn extends Map and other interfaces. Hence the MultiMap cannot have a get method
which returns a Collection instead of Object as that would mean implementing IterableMap with
the Generics set to be <K,Collection<V>>. In that case the put method's signature
would become
> public Collection<V> put(K key, Collection<V> value); 
> which we do not want.The same problem would arise with other methods as well, ex: containsValue
> My proposal is why carry on the signatures of a Map and put it on MultiMap. Where as
I do agree that it is a Map after all and has very similar implementation and functionality,
it is very different at other levels. And even though the MultiMap interface supports generics,
the methods are not strongly typed, which defeats the purpose of having generics. So why can't
we have a separate set of interfaces for MultiMap which do not extend Map. That way we can
have strongly typed methods on the MultiMap.
> I have included a a patch for these changes. It is not fully complete and has some gaps
in some TestCases and the documentation but gives a fairly good idea of what I am talking
about. Please let me know your thoughts on taking this approach. Then i will improve the implementation
and submit another patch.
> The other way could be that we let MultiMap extend the interfaces it does today, but
with proper types rather than Object. I mean something like this
> public interface MultiMap<K,V> extends IterableMap<K,Collection<V>>
instead of 
> public interface MultiMap<K,V> extends IterableMap<K,Object>
> And then have a separate set of methods on the MultiMap interface which supports the
specific MultiMap functionality. For example, the put method with the above implementation
would become 
> Collection<V>	put(K key, Collection<V> value)
> and we can have another method as 
> V putValue(K key, V value)
> This way the functionality of Map is preserved along with strongly typed MultiMap methods.
If you feel that this approach is better than the earlier one, i will implement the same and
submit a patch

This message was sent by Atlassian JIRA

View raw message