kinow commented on code in PR #748:
URL: 
https://github.com/apache/commons-collections/pull/748#discussion_r4308225870


##########
src/main/java/org/apache/commons/collections4/MultiValuedMap.java:
##########
@@ -181,6 +181,11 @@ default MultiValuedMap<V, K> inverted() {
      * <p>
      * A map iterator is an efficient way of iterating over maps. There is no 
need to access the entries collection or use {@code Map.Entry} objects.
      * </p>
+     * <p>
+     * The iterator visits one key-value mapping at a time, so a key with 
several values is returned once per value. The implementations in this
+     * library do not support {@link MapIterator#setValue(Object)} and throw 
an {@link UnsupportedOperationException} from it; to replace a value,

Review Comment:
   I think this is saying that `setValue` is definitely not supported.
   
   But in reality it is, only depends on whether the implementations of 
commons-collections or other implementations support the `setValue`. Right now 
we may not have a class that supports it, but it may change...
   
   Maybe we can just say here in one new paragraph that some operations of the 
map iterator are optional (e.g., `remove`, `setValue`) and users must refer to 
that interface and implementation's javadocs for details.



##########
src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java:
##########
@@ -344,6 +344,14 @@ public void remove() {
             it.remove();
         }
 
+        /**
+         * Always throws {@link UnsupportedOperationException}, as the entries 
of a multivalued map are read-only.

Review Comment:
   Elsewhere in this code I think we have `MultiValuedMap` and MultiValuedMap 
(formatted and not), in the Javadocs. Maybe stick to those, instead of this 
lower-case version? And the always is technically not correct, I think, as 
there are two `@throws`?



##########
src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java:
##########
@@ -747,6 +755,13 @@ public Set<K> keySet() {
         return getMap().keySet();
     }
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * The returned iterator does not support {@link 
MapIterator#setValue(Object)}: the entries it iterates are read-only, so {@code 
setValue}
+     * throws an {@link UnsupportedOperationException}.
+     * </p>
+     */

Review Comment:
   👍 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to