Copilot commented on code in PR #718:
URL:
https://github.com/apache/commons-collections/pull/718#discussion_r3665156861
##########
src/test/java/org/apache/commons/collections4/SplitMapUtilsTest.java:
##########
@@ -116,12 +116,14 @@ void testReadableMap() {
assertEquals(other, map);
assertEquals(other.hashCode(), map.hashCode());
- // remove
- for (int i = 0; i < 10; i++) {
- assertEquals(i, map.remove(String.valueOf(i)).intValue());
- assertEquals(--sz, map.size());
- }
- assertTrue(map.isEmpty());
+ // remove, and the Map default methods that route through it
+ attemptPutOperation(() -> map.remove("0"));
+ attemptPutOperation(() -> map.remove("1", 1));
+ attemptPutOperation(() -> map.computeIfPresent("2", (k, v) -> null));
+ attemptPutOperation(() -> map.merge("3", 3, (a, b) -> null));
+
+ assertEquals(sz, map.size());
+ assertEquals(sz, backingMap.size());
Review Comment:
The PR description calls out `compute(...)` as also mutating via the
`remove` path, but the test currently covers `computeIfPresent(...)` and
`merge(...)` only. Add an assertion that `map.compute(\"<existingKey>\", (k, v)
-> null)` throws and does not change `map`/`backingMap` sizes, to ensure the
reported hole is fully guarded against regressions.
##########
src/main/java/org/apache/commons/collections4/SplitMapUtils.java:
##########
@@ -131,9 +131,15 @@ public void putAll(final Map<? extends K, ? extends V>
map) {
throw new UnsupportedOperationException();
}
+ /**
+ * Always throws {@link UnsupportedOperationException}.
+ *
+ * @param key Ignored.
+ * @throws UnsupportedOperationException Always thrown.
+ */
@Override
public V remove(final Object key) {
- return get.remove(key);
+ throw new UnsupportedOperationException();
Review Comment:
The new Javadoc says `@param key Ignored.`, which is a bit misleading for a
`Map` method signature. Consider either using `{@inheritDoc}` and adding a
short note that the operation is unsupported, or describing `key` as the key
whose mapping would be removed (even though the method always throws).
--
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]
