Hi Matt, Thanks for the info. I have a Jira issue created for this ( https://issues.apache.org/jira/browse/COLLECTIONS-508). I have attached the patch there.
Thanks Dipanjan On Sun, Mar 2, 2014 at 10:21 PM, Matt Benson <gudnabr...@gmail.com> wrote: > 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 <dipanja...@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 <dipanja...@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 <gudnabr...@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" <dipanja...@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.neidh...@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 < > dipanja...@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 < > >>> dipanja...@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.neidh...@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-unsubscr...@commons.apache.org > >>> > > > >> For additional commands, e-mail: dev-h...@commons.apache.org > >>> > > > >> > >>> > > > >> > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >> > >> > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > >
