commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Benson <gudnabr...@gmail.com>
Subject Re: [collections] Issues with MultiMap generics and need of a MultiTrie
Date Sun, 02 Mar 2014 16:51:20 GMT
Apache lists generally don't allow attachments. Please raise a JIRA issue
and attach there.

Matt


On Sun, Mar 2, 2014 at 10:46 AM, Dipanjan Laha <dipanjan21@gmail.com> wrote:

> Hi Thomas,
>
> I have implemented the MultiValuedMap interface, the MultiValuedHashMap
> and a MultiValuedHashMapTest as per the discussions. I haven't completed
> the documentation yet. If the implementations look fine, I will add the
> remaining documentations.
>
> A few more points regarding the implementation
>
> 1. I have added a few methods to the MultiValuedMap interface which were
> not there in the MultiMap. I think they would be a good addition to the
> interface IMHO. They are
>     boolean containsValue(Object key, Object value);
>     int totalSize();
>     void putAll(MultiValuedMap<? extends K, ? extends V> map);
> 2. I have added an AbstractMultiValuedMapDecoractor on the lines
> of AbstractMapDecorator, which can be extended by other MultiValuedMap
> implementations like say a MultiValuedTreeMap
> 3. I have created MultiValuedGet and MultiValuedPut to honor the Get/Put
> split concepts. It was not possible for MultiValuedMap to extend the Get &
> Put directly due to the limitations I had mentioned in my earlier mail.
> 4. I have marked the incomplete documentations with TODO tags.
>
> PFA the patch for the new Classes. Please go through the implementation
> and let me know if I missed some thing or if things need to be done in some
> other way.
>
> Regards
> Dipanjan
>
>
>
> On Wed, Feb 26, 2014 at 8:04 PM, Dipanjan Laha <dipanjan21@gmail.com>wrote:
>
>> Thanks for pointing this out. However, implementing Get & Put directly
>> would pose the following problems.
>>
>> If interface MultiValuedMap<K,V> extends Get<K,Collection<V>>
>>
>> the method "values" would be forced to have a signature of
>>
>> Collection<Collection<V>> values()
>>
>> whereas we would want
>>
>> Collection<V> values().
>>
>> This wont be possible as we would extend Get with the generics
>> <K,Collection<V>> as we want the method "get" to have a signature like
>>
>> Collection<V> get(Object key)
>>
>> Now, extending the Put interface with generics <K,V> does not pose that
>> much of an issue except that the Map interface in Java 7 has a put
>> signature as V put(K key, V value)  whereas the Collections 4 Put still has
>> Object put(Key k, V value), but we can ignore this if we want.
>>
>> For the problem with Get, we can have a parallel MultiValuedGet and
>> MultiValuedPut interfaces to honor the Get/Put split concepts. Although we
>> don't really need the MultiValuedPut, we can have that for consistency.
>>
>> Let me know your thoughts on this.
>>
>> Regards
>> Dipanjan
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Wed, Feb 26, 2014 at 7:12 PM, Matt Benson <gudnabrsam@gmail.com>wrote:
>>
>>> Don't forget about the Get/Put/split map concepts from Collections 4. It
>>> would seem you could implement those interfaces and provide that amount
>>> of
>>> abstraction anyway.
>>>
>>> Matt
>>> On Feb 26, 2014 3:26 AM, "Dipanjan Laha" <dipanjan21@gmail.com> wrote:
>>>
>>> > Hi Thomas,
>>> >
>>> > This sounds great. Moving MultiKeyMap to the new package does sound
>>> like
>>> > the way to go ahead. I will start with the implementation of the
>>> interface
>>> > and the MultiValuedHashMap. I should be able to submit a patch with a
>>> basic
>>> > implementation and some test cases by the end of this week. I can then
>>> > modify and incorporate changes as per your review and suggestions.
>>> >
>>> > Regards
>>> > Dipanjan
>>> >
>>> >
>>> > On Wed, Feb 26, 2014 at 2:32 PM, Thomas Neidhart
>>> > <thomas.neidhart@gmail.com>wrote:
>>> >
>>> > > Hi Dipanjan,
>>> > >
>>> > > I was thinking about a name for the new interface, but I actually
>>> like
>>> > your
>>> > > proposal of MultiValuedMap.
>>> > >
>>> > > For the package, I think we can stick with multimap, and at some
>>> point we
>>> > > could also move the MultiKeyMap there, which would be logical imho.
>>> > >
>>> > > The implementation names are also sound.
>>> > >
>>> > > Thomas
>>> > >
>>> > >
>>> > >
>>> > >
>>> > > On Wed, Feb 26, 2014 at 9:28 AM, Dipanjan Laha <dipanjan21@gmail.com
>>> >
>>> > > wrote:
>>> > >
>>> > > > Hi Thomas,
>>> > > >
>>> > > > It would be great if we can start the discussion on the new
>>> interface
>>> > for
>>> > > > MultiMap and a new package for the implementations as suggested
by
>>> you.
>>> > > > Then I'll be able to put some code together for the same.
>>> > > >
>>> > > > IMO we can have
>>> > > >
>>> > > > 1. New Interface for MultiMap with the name MultiValuedMap or
>>> > MultiValMap
>>> > > >  (as MultiValueMap is already the existing implementing class).
>>> > > > 2. New package for the implementations:
>>> > > > org.apache.commons.collections.multimap or
>>> > > > org.apache.commons.collections.multivaluedmap
>>> > > > 3. Implementation names like : MultiValuedHashMap,
>>> MultiValuedTreeMap
>>> > etc
>>> > > >
>>> > > > Please let me know of your thoughts on these.
>>> > > >
>>> > > > Regards
>>> > > > Dipanjan
>>> > > >
>>> > > > On Sun, Feb 23, 2014 at 8:36 PM, Dipanjan Laha <
>>> dipanjan21@gmail.com>
>>> > > > wrote:
>>> > > >
>>> > > > > Hi Thomas,
>>> > > > >
>>> > > > > Thanks for your feedback. I created an improvement request
in
>>> Jira
>>> > for
>>> > > > the
>>> > > > > same (https://issues.apache.org/jira/browse/COLLECTIONS-508
)
>>> as I
>>> > > > > thought it could be better tracked there. Sorry for the
>>> duplication
>>> > in
>>> > > > the
>>> > > > > mail list and Jira. I have also attached a patch in Jira
where I
>>> have
>>> > > > > modified the existing MultiMap interface and the MultiValueMap
>>> > > > > implementation and their test cases. I agree that it would
break
>>> > > backward
>>> > > > > compatibility and we should go with your suggestion of
>>> deprecating
>>> > the
>>> > > > > existing ones and design fresh interfaces for the same. The
>>> patch is
>>> > > > just a
>>> > > > > sample implementation to demonstrate the issue and is far
from
>>> being
>>> > > > > complete in terms of documentation and test cases. I am also
>>> > attaching
>>> > > > the
>>> > > > > patch here for your reference.
>>> > > > >
>>> > > > > Please go through the patch and also let me know of your
>>> thoughts on
>>> > > how
>>> > > > > we should proceed with the new interface and package structure.
>>> I'll
>>> > be
>>> > > > > happy to change and redirect the implementation as per your
>>> > > suggestion. I
>>> > > > > am new to Apache Commons, but with some guidance I should
not
>>> have
>>> > > issues
>>> > > > > implementing them to start with.
>>> > > > >
>>> > > > > As for MultiTrie, as you mentioned, we can start with it
once
>>> the new
>>> > > > > MultiMap has been finalized.
>>> > > > >
>>> > > > > Regards
>>> > > > > Dipanjan
>>> > > > >
>>> > > > >
>>> > > > >
>>> > > > >
>>> > > > > On Sun, Feb 23, 2014 at 6:09 PM, Thomas Neidhart <
>>> > > > > thomas.neidhart@gmail.com> wrote:
>>> > > > >
>>> > > > >> On 02/22/2014 02:00 PM, Dipanjan Laha wrote:
>>> > > > >> > Hello,
>>> > > > >>
>>> > > > >> Hi Dipanjan,
>>> > > > >>
>>> > > > >> > Recently I had the need of using a MultiMap in one
of my
>>> > projects. I
>>> > > > >> found
>>> > > > >> > that commons collection already has a MultiMap interface
and
>>> an
>>> > > > >> > implementation.
>>> > > > >> >
>>> > > > >> > 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 extending 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 method.
>>> > > > >> >
>>> > > > >> > 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.
>>> > > > >>
>>> > > > >> The MultiMap interface as it is right now is flawed,
and should
>>> have
>>> > > > >> been cleaned up prior to the 4.0 release imho (and I
regretted
>>> it
>>> > > > >> already before your post).
>>> > > > >>
>>> > > > >> As you correctly pointed out, the problem comes from
the fact
>>> that
>>> > it
>>> > > > >> extends Map<K, Object> which leads to problems
once generics
>>> have
>>> > been
>>> > > > >> introduced (before it did not matter that much as you
had to
>>> cast
>>> > > > >> anyway, as it is also documented in the javadoc).
>>> > > > >>
>>> > > > >> One mitigation for this was the introduction of this
method to
>>> > > > >> MultiValueMap, but it is clearly not enough:
>>> > > > >>
>>> > > > >>  public Collection<V> getCollection(Object key)
>>> > > > >>
>>> > > > >>
>>> > > > >> Unfortunately, it is not easy to fix this now after collections
>>> 4.0
>>> > > has
>>> > > > >> been released. We need to keep backwards compatibility,
but we
>>> could
>>> > > do
>>> > > > >> the following:
>>> > > > >>
>>> > > > >>  * deprecate the existing interfaces/classes:
>>> > > > >>    - MultiMap
>>> > > > >>    - MultiValueMap
>>> > > > >>
>>> > > > >>  * design a new, clean interface (by not extending Map)
>>> > > > >>  * add new package multimap with concrete implementations
for
>>> > > different
>>> > > > >>    types of maps (right now only hashmaps are supported)
>>> > > > >>
>>> > > > >> > Please let me know your thoughts on this. I can
submit a
>>> patch for
>>> > > > these
>>> > > > >> > changes based on your feedback. One more thing,
I also am in
>>> need
>>> > > of a
>>> > > > >> > MultiTrie which is currently not there. I am implementing
the
>>> same
>>> > > by
>>> > > > >> > wrapping PatriciaTrie. Now I am a bit confused here
as, if I
>>> make
>>> > > the
>>> > > > >> > MultiTrie interface on the lines of MultiMap, it
would have
>>> the
>>> > same
>>> > > > >> > limitations. In that case I was planning to have
a separate
>>> set of
>>> > > > >> > interfaces for MultiTrie which does not extend any
other
>>> > interface.
>>> > > > And
>>> > > > >> in
>>> > > > >> > case, we do change the MultiMap interface to be
independent of
>>> > Map,
>>> > > > then
>>> > > > >> > MultiTrie can extend MultiMap. Please let me know
your
>>> thoughts on
>>> > > > this
>>> > > > >> as
>>> > > > >> > well as I am implementing the same for my project
right now
>>> and
>>> > > would
>>> > > > >> like
>>> > > > >> > to contribute it back to the commons collection.
>>> > > > >>
>>> > > > >> Patches are always welcome, but we first need a decision
in
>>> which
>>> > > > >> direction to go, see above.
>>> > > > >>
>>> > > > >> Regarding the MultiTrie:
>>> > > > >>
>>> > > > >> Indeed, it is the same problem, so it should go hand
in hand
>>> with
>>> > the
>>> > > > >> revamp of the MultiMap interface.
>>> > > > >>
>>> > > > >> Thomas
>>> > > > >>
>>> > > > >>
>>> > ---------------------------------------------------------------------
>>> > > > >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> > > > >> For additional commands, e-mail: dev-help@commons.apache.org
>>> > > > >>
>>> > > > >>
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

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