commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dipanjan Laha <dipanja...@gmail.com>
Subject Re: [collections] MultiValuedMap interface discussion
Date Fri, 28 Mar 2014 08:37:52 GMT
Thanks Ted for pointing to this.

I checked out the guavas Multimap and if I understand correctly, they are
returning a wrapped Collection with an internal reference to the Multimap.
The basic behavior is
 - When add is triggered on the collection, they create the mapping in
their Multimap if it is not there
 - When remove is triggered and the last element is removed, the mapping is
removed.

We did discuss this approach, but Thomas raised a concern that there can be
undefined behavior if get is called twice before add. I spent some time on
this and imho I think that there won't be any problem if we always return a
wrapped Collection irrespective of whether the mapping is present or not.
So for every add or remove on it, it would check the underlying map and
then either add or remove the mapping if necessary or just delegate the
call to the decorated collection if the mappings need not be altered.

Also, we are currently not removing the mapping in case
get(key1).remove(value2) removes the last element in the collection. This
is wrong imho as we do remove the mapping when remove is called on the
values view. Now to fix this, we would need to return such a wrapped
Collection from get anyway, and there we can add the above mentioned
behavior to the add method when a mapping is not present.

What do you guys think?


On Fri, Mar 28, 2014 at 5:17 AM, Ted Dunning <ted.dunning@gmail.com> wrote:

> Following Guava on this has something to be said for it.
>
> https://code.google.com/p/guava-libraries/wiki/NewCollectionTypesExplained
>
> Their decision is that Multimap#get returns a collection always.  If there
> are no values, then an empty collection is returned so that you can always
> do
>
>       m.get(key).size()
>
> or
>
>      m.get(key).add(foo)
>
> The value returned is a magical view which only takes up space on demand so
> there is little consing done.  There is an asMap method for which get will
> return null on missing keys.
>
>
>
>
> On Thu, Mar 27, 2014 at 2:55 PM, Paul Benedict <pbenedict@apache.org>
> wrote:
>
> > The downside of it returning an empty collection is you either have (1)
> to
> > instantiate a collection just to say you have nothing or (2) you use an
> > immutable collection. #1 is bad in itself and #2 is only as bad if the
> > collection is otherwise writable. For example, it would be really strange
> > for the returned collection to be mutable if you have something but
> > immutable if you have nothing.
> >
> > My preference is you return null. That's the most rational answer, imo.
> >
> >
> > On Thu, Mar 27, 2014 at 4:44 PM, Thomas Neidhart
> > <thomas.neidhart@gmail.com>wrote:
> >
> > > Hi,
> > >
> > > we are currently working on a new MultiValuedMap interface for
> > > collections, see https://issues.apache.org/jira/browse/COLLECTIONS-508
> .
> > >
> > > During the work we stumbled across an issue we would like to discuss.
> > > The MultiValuedMap is basically a Map that can hold multiple values
> > > associated to a given key. Thus the get(K key) method will normally
> > > return a Collection.
> > >
> > > In case no mapping for the key is stored in the map, it may either
> > > return null (like a normal map), or an empty collection.
> > >
> > > I would be in favor to define that get() always returns a collection
> and
> > > never returns null. The advantage being that the result of get() can
> > > safely be used for further operations, e.g. size(), iterator(), ...
> > > keeping the interface of MultiValuedMap smaller and simple (i.e. no
> need
> > > to add additional methods there like size(K key) or iterator(K key)).
> > >
> > > The containsKey method would have to check if there is either no
> mapping
> > > at all for the key or the stored collection is empty:
> > >
> > > public boolean containsKey(K key) {
> > >   Collection coll = decoratedMap().get(key);
> > >   return coll != null && coll.size > 0;
> > > }
> > >
> > > The downside would be that read operations may also alter the map thus
> > > leading to unexpected ConcurrentModificationExceptions when iterating
> on
> > > e.g. value().
> > >
> > > So, I would be interested on opinions about this.
> > >
> > > Thomas
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> >
> >
> > --
> > Cheers,
> > Paul
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message