You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "paulk-asert (via GitHub)" <gi...@apache.org> on 2025/12/12 11:29:18 UTC
[PR] provide an inverse method for multimaps [commons-collections]
paulk-asert opened a new pull request, #665:
URL: https://github.com/apache/commons-collections/pull/665
This PR adds an inverse method into the following multimap implementations:
`ArrayListValuedHashMap`
`ArrayListValuedLinkedHashMap`
`HashSetValuedHashMap`
`HashSetValuedLinkedHashMap`
There is also an `inverseInto` method added to `MultiMapUtils` to handle the case of a heterogenous collector.
As well as a test case to the relevant source files. Decorated multimaps throw `UnsupportedOperationException` by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2621414983
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
Review Comment:
Okay, the intention is that when calling `inverted` you have the following input -> output class relationships:
ArrayListValuedHashMap -> ArrayListValuedHashMap
ArrayListValuedLinkedHashMap -> ArrayListValuedLinkedHashMap
LinkedHashSetValuedLinkedHashMap -> LinkedHashSetValuedLinkedHashMap
LinkedHashSetValuedHashMap -> LinkedHashSetValuedHashMap
I'll try to think of better wording.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618587063
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,17 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse MultiValuedMap with inverted mappings.
Review Comment:
'the' => 'a new'
'the' implies that there is only one such map
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param dest add value-to-key mappings here
+ * @param <K> the dest key type
+ * @param <V> the dest value type
+ * @param <M> the dest multimap
+ * @return the destination
Review Comment:
This does not make sense to me. What is the destination?
I think the Javadoc needs to make clear that the method creates a new map with the inverted keys
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2619784014
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
Review Comment:
Does it have to be a MultiValuedMap? Would an ordinary Map be OK?
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
Review Comment:
What are the valid types for M?
Are all multimap types suitable?
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
Review Comment:
'same kind' is ambiguous.
AIUI, this method returns a MultiValuedMap; is that not the 'same kind' of map as the original?
It's not clear when one should consider using #inverted()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618867244
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param dest add value-to-key mappings here
+ * @param <K> the dest key type
+ * @param <V> the dest value type
+ * @param <M> the dest multimap
+ * @return the destination
Review Comment:
The utility method doesn't create a new multimap but stores inverted mappings in the supplied collector (destination) which is also returned. The `inverted` method implementations supply a new multimap as the collector, but it doesn't need to be the same type or be new (to allow aggregation) when using the utility method. I could try to use "collector" everywhere and avoid "destination" if you think that would be clearer?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2619549490
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param dest add value-to-key mappings here
+ * @param <K> the dest key type
+ * @param <V> the dest value type
+ * @param <M> the dest multimap
+ * @return the destination
Review Comment:
updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] provide an inverse method for multimaps [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2616558853
##########
src/main/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMap.java:
##########
@@ -118,6 +119,13 @@ protected ArrayList<V> createCollection() {
return new ArrayList<>(initialListCapacity);
}
+ @Override
+ public ArrayListValuedHashMap<V, K> inverse() {
+ final ArrayListValuedHashMap<V, K> result = new ArrayListValuedHashMap<>();
+ MultiMapUtils.inverseInto(this, result);
+ return result;
Review Comment:
agreed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] provide an inverse method for multimaps [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2616558801
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,16 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse of this MultiValuedMap. The inverse has a value-to-key mapping
+ * for each key-to-value mapping in the original.
+ *
+ * @return the inverse MultiValuedMap
+ */
+ default MultiValuedMap<V, K> inverse() {
Review Comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618940361
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,17 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse MultiValuedMap with inverted mappings.
Review Comment:
Yes, it is the name of the abstraction used to represent a multimap, but I understand where you are coming from and I'll see what improvement to the wording I can think of.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2621400525
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
+ * @return the updated collector
+ */
+ public static <K, V, M extends MultiValuedMap<K, V>>
+ M invertInto(MultiValuedMap<? extends V, ? extends K> source, M collector) {
Review Comment:
Guava has similar methods to what is proposed here. It uses `invertFrom` as the utility method and `inverse` as the instance method. I think `invert` and `invertMerge` have some benefits. I'll ponder a change and convert `collector` to `target` or similar. Hmmm.. `CollectionUtils#collect` uses `inputCollection` and `outputCollection`. Maybe that is the way to go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2621377853
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
Review Comment:
You could write a method that worked with ordinary maps. Given that ordinary maps don't need the value to be a collection, there are multiple possible semantics. One potential behavior would be to throw a runtime exception if called with a map having a value that isn't a collection. Another semantic might coerce the value into a singleton collection. Other libraries tend to have factory methods for creating a multimap from an ordinary map - different factory methods could provide different semantics. Then the invert method just works with multimaps. Commons Collections already has such factory methods, e.g. `HashSetValuedHashMap(Map)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2619548286
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,17 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse MultiValuedMap with inverted mappings.
Review Comment:
updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] provide an inverse method for multimaps [commons-collections]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #665:
URL: https://github.com/apache/commons-collections/pull/665#issuecomment-3646393468
Thank you @paulk-asert
After this PR, I was able to raise the bar on our coverage checks to:
```xml
<commons.jacoco.haltOnFailure>true</commons.jacoco.haltOnFailure>
<commons.jacoco.classRatio>1.00</commons.jacoco.classRatio>
<commons.jacoco.instructionRatio>0.99</commons.jacoco.instructionRatio>
<commons.jacoco.methodRatio>0.99</commons.jacoco.methodRatio>
<commons.jacoco.branchRatio>0.95</commons.jacoco.branchRatio>
<commons.jacoco.lineRatio>0.99</commons.jacoco.lineRatio>
<commons.jacoco.complexityRatio>0.97</commons.jacoco.complexityRatio>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2629469852
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,29 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from an inputMultimap
+ * and add them to an outputMultimap. Some {@code MultiValuedMap}
+ * implementations support the {@link MultiValuedMap#inverted()} method.
+ * These typically return a new multimap instance with the same class as the original.
+ * Consider using this method instead if you wish to supply a particular
Review Comment:
changed
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,29 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from an inputMultimap
+ * and add them to an outputMultimap. Some {@code MultiValuedMap}
Review Comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2619710579
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
+ * @return the updated collector
+ */
+ public static <K, V, M extends MultiValuedMap<K, V>>
+ M invertInto(MultiValuedMap<? extends V, ? extends K> source, M collector) {
Review Comment:
Hi @paulk-asert
A few items here:
- I'm wondering if a simpler name like `invert` would be better since we don't usually see `Into` in method names. We know from the param names what the source and target are. Using "Into" doesn't add value for me.
- The parameter name `collector` might be confusing because a `Collector` in Java is something else but kind of related. Since we have `source`, having `dest`, `destination` or `target` would be better IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618912649
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param dest add value-to-key mappings here
+ * @param <K> the dest key type
+ * @param <V> the dest value type
+ * @param <M> the dest multimap
+ * @return the destination
Review Comment:
I think collector would be clearer here.
For the return comment, it could say 'the updated collector'
Also it would help to document the valid types for source and collector.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] provide an inverse method for multimaps [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #665:
URL: https://github.com/apache/commons-collections/pull/665#issuecomment-3646233579
> Hi @paulk-asert
> Thank you for the PR.
> You'll want to run 'mvn' by itself and fix errors before you push. That will run the default goal which includes all our checks.
@garydgregory Should be fixed. Sorry for the earlier noise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2619760021
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
+ * @return the updated collector
+ */
+ public static <K, V, M extends MultiValuedMap<K, V>>
+ M invertInto(MultiValuedMap<? extends V, ? extends K> source, M collector) {
Review Comment:
Maybe invertMerge would better describe what the method does?
Also rather than source and dest, would input and output be clearer?
I don't like the abbreviation 'dest'; it's best to spell names in full as not all users will have English as a first language.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2623218441
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,29 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from an inputMultimap
+ * and add them to an outputMultimap. Some {@code MultiValuedMap}
+ * implementations support the {@link MultiValuedMap#inverted()} method.
+ * These typically return a new multimap instance with the same class as the original.
+ * Consider using this method instead if you wish to supply a particular
Review Comment:
'this method' is ambiguous here: does it refer to the method inverted() which has just been described, or the method in this class?
I think it would be clearer to describe the usage for the current class method before explaining that there are alternatives.
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,29 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from an inputMultimap
+ * and add them to an outputMultimap. Some {@code MultiValuedMap}
Review Comment:
inputMultimap => input MultiValuedMap
outputMultimap => output MultiValuedMap
inputMultimap and outputMultimap are not Java types, and AFAICT they must both be MultiValuedMap, rather than just Multimap
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2629470244
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,29 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from an inputMultimap
+ * and add them to an outputMultimap. Some {@code MultiValuedMap}
+ * implementations support the {@link MultiValuedMap#inverted()} method.
+ * These typically return a new multimap instance with the same class as the original.
+ * Consider using this method instead if you wish to supply a particular
+ * type of multimap or wish to merge multiple multimap inverse mappings into a single multimap.
+ *
+ * @param inputMultimap take key-to-value mappings from here
+ * @param outputMultimap add value-to-key mappings here
+ * @param <K> the outputMultimap key type
+ * @param <V> the outputMultimap value type
+ * @param <M> the outputMultimap multimap with key and value types reversed compare to inputMultimap
Review Comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2619586276
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
Review Comment:
The Javadoc is not entirely clear that the inverted mappings are added to the collector, rather than replacing whatever was there originally. I think it can be read either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #665:
URL: https://github.com/apache/commons-collections/pull/665
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2623184640
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,29 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from an inputMultimap
+ * and add them to an outputMultimap. Some {@code MultiValuedMap}
+ * implementations support the {@link MultiValuedMap#inverted()} method.
+ * These typically return a new multimap instance with the same class as the original.
+ * Consider using this method instead if you wish to supply a particular
+ * type of multimap or wish to merge multiple multimap inverse mappings into a single multimap.
+ *
+ * @param inputMultimap take key-to-value mappings from here
+ * @param outputMultimap add value-to-key mappings here
+ * @param <K> the outputMultimap key type
+ * @param <V> the outputMultimap value type
+ * @param <M> the outputMultimap multimap with key and value types reversed compare to inputMultimap
Review Comment:
compare to => compared with
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2621400525
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
+ * @return the updated collector
+ */
+ public static <K, V, M extends MultiValuedMap<K, V>>
+ M invertInto(MultiValuedMap<? extends V, ? extends K> source, M collector) {
Review Comment:
Guava has similar methods to what is proposed here. It uses `invertFrom` as the utility method and `inverse` as the instance method. I think `invert` and `invertMerge` have some benefits. I'll ponder a change and convert `collector` to `target` or similar. Hmmm.. `CollectionUtils#collect` uses `inputCollection` and `outputCollection`. Maybe that convention is the way to go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] provide an inverse method for multimaps [commons-collections]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #665:
URL: https://github.com/apache/commons-collections/pull/665#issuecomment-3646128010
Hi @paulk-asert
Thank you for the PR.
You'll want to run 'mvn' by itself and fix errors before you push. That will run the default goal which includes all our checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618949761
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param dest add value-to-key mappings here
+ * @param <K> the dest key type
+ * @param <V> the dest value type
+ * @param <M> the dest multimap
+ * @return the destination
Review Comment:
Can you clarify what you are after for source and collector? The types/generics information isn't sufficient?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2622127731
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
+ * @return the updated collector
+ */
+ public static <K, V, M extends MultiValuedMap<K, V>>
+ M invertInto(MultiValuedMap<? extends V, ? extends K> source, M collector) {
Review Comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2622157889
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
+ * @param source take key-to-value mappings from here
+ * @param collector add value-to-key mappings here
+ * @param <K> the collector key type
+ * @param <V> the collector value type
+ * @param <M> the collector multimap
Review Comment:
I changed the wording. Let me know if it makes sense or still sounds confusing to you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2622158509
##########
src/main/java/org/apache/commons/collections4/MultiMapUtils.java:
##########
@@ -170,6 +171,27 @@ public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
return map == null || map.isEmpty();
}
+ /**
+ * A utility method to invert the mappings from a source MultiValuedMap
+ * to a collector MultiValuedMap. If you are trying to create an
+ * inverse multimap of the same kind as the original, consider using
+ * {@link MultiValuedMap#inverted()}.
+ *
Review Comment:
wording changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618879568
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,17 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse MultiValuedMap with inverted mappings.
Review Comment:
In mathematical jargon, the thing being returned is the "inverse multimap". I see what you are saying that those not knowing the math jargon might confuse that wording with meaning just one. Then again, totally forgoing the math jargon might be less clear for those that are aware of the term. I'll try to think of a middle ground.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add MultiValuedMap.inverted() [commons-collections]
Posted by "sebbASF (via GitHub)" <gi...@apache.org>.
sebbASF commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2618920793
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,17 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse MultiValuedMap with inverted mappings.
Review Comment:
The comment refers to 'the MultiValueMap' which is a class, not a mathematical concept.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] provide an inverse method for multimaps [commons-collections]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #665:
URL: https://github.com/apache/commons-collections/pull/665#discussion_r2616528029
##########
src/main/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMap.java:
##########
@@ -118,6 +119,13 @@ protected ArrayList<V> createCollection() {
return new ArrayList<>(initialListCapacity);
}
+ @Override
+ public ArrayListValuedHashMap<V, K> inverse() {
+ final ArrayListValuedHashMap<V, K> result = new ArrayListValuedHashMap<>();
+ MultiMapUtils.inverseInto(this, result);
+ return result;
Review Comment:
No need to be so verbose here and for all implementations IMO:
```suggestion
return MultiMapUtils.inverseInto(this, new ArrayListValuedHashMap<>());
```
##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -140,6 +140,16 @@ public interface MultiValuedMap<K, V> {
*/
Collection<V> get(K key);
+ /**
+ * Returns the inverse of this MultiValuedMap. The inverse has a value-to-key mapping
+ * for each key-to-value mapping in the original.
+ *
+ * @return the inverse MultiValuedMap
+ */
+ default MultiValuedMap<V, K> inverse() {
Review Comment:
Java 21 has `java.util.List.reversed()`, so how about `inverted()` to match the same naming pattern?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org