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

ImmutableMap.Builder should have remove method #3596

Closed
dapengzhang0 opened this issue Sep 6, 2019 · 5 comments
Closed

ImmutableMap.Builder should have remove method #3596

dapengzhang0 opened this issue Sep 6, 2019 · 5 comments

Comments

@dapengzhang0
Copy link
Member

Without Builder.remove() method, it will be very inefficient to create a new immutable map from an existing immutable map removing a key:

Map newMapWithoutKey1 = ImmutableMap.builder()
    .putAll(map)
    .remove("key1")
    .build();
@kluever
Copy link
Member

kluever commented Sep 6, 2019

In general, we recommend mutating a mutable map type instead of trying to pretend a builder is a mutable map.

So, use a LinkedHashMap or HashMap, perform your additions/removals, and then ImmutableMap.copyOf(mutableMap)

@kluever kluever closed this as completed Sep 6, 2019
@dapengzhang0
Copy link
Member Author

dapengzhang0 commented Sep 6, 2019

Then one has to make a mutable copy of an existing map, even the original map is mutable, because we may not want the original map to be mutated.

Map copy = new LinkedHashMap(map);
copy.remove("key1");
Map newMapWithoutKey1 = ImmutableMap.copyOf(copy);

@kluever
Copy link
Member

kluever commented Sep 6, 2019

That seems fine to me.

If you're super worried about performance, you can add the entries from the existing mutable map to the ImmutableMap builder in a loop, and just don't add the entry if it has a certain key (the key you want to "remove").

Or, alternatively, use a lazy Maps.filterKeys() and Predicate.not(badKey), wrapped in ImmutableMap.copyOf().

@dapengzhang0
Copy link
Member Author

Thanks. It is fine to me as well.

If I'm supper worried about performance, I have to use ImmutableMap.builderWithExpectedSize() before adding entries in a loop, otherwise the performance is almost the same as making a mutable copy. Is my understanding correct?

@kluever
Copy link
Member

kluever commented Sep 6, 2019

That sounds approximately correct, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants