Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[COLLECTIONS-663] Update AbstractMultiValuedMap asMap() #110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dota17
Copy link
Contributor

@dota17 dota17 commented Nov 6, 2019

[COLLECTIONS-663] https://issues.apache.org/jira/browse/COLLECTIONS-663
Hi:
I think the representation of Multimap is {1: A, 1: B, 2: C}. So when A and B are deleted, only {2: C} is left. And asMap() method returns a map in the form of {1:[A, B], 2:[C]}. In this map, {1:[], 2:[C]} is left when A and B are deleted. So wrappedCollection(key) should not be used。 It willl show as {1: A, 1: B, 2: C} when wrappedCollection(key) is used, and the asMap() method does not make sense.

Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my inline comment, other assert changes look good. TY.

@@ -879,7 +879,8 @@ public boolean remove(final Object o) {
public Map.Entry<K, Collection<V>> next() {
final Map.Entry<K, Collection<V>> entry = super.next();
final K key = entry.getKey();
return new UnmodifiableMapEntry<>(key, wrappedCollection(key));
final Collection<V> value = entry.getValue();
return new UnmodifiableMapEntry<>(key, value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just inline key and value IMO.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 90.034% when pulling 58ec415 on dota17:AbstractMultiValuedMap into a3eb3be on apache:master.

@dota17 dota17 requested a review from garydgregory May 20, 2020 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants