6

In the following piece of code:

if (map.containsKey(key)) {
    map.remove(key);
}

Looking at performance, is it useful to first do a Map.containsKey() check before trying to remove the value from the map?

Same question goes for retrieving values, is it useful to first do the contains check if you know that the map contains no null values?

if (map.containsKey(key)) {
    Object value = map.get(key);
}
2
  • 1
    The first example is redundant even in the presence of null keys. Feb 10, 2015 at 11:46
  • Which JDK implementation?
    – aioobe
    Feb 10, 2015 at 11:47

4 Answers 4

6

remove returns null if there's no mapping for key no exception will be thrown:

public V remove(Object key)

I don't see any reason to perform that if before trying to remove a key, perhaps maybe if you want to count how many items where removed from the map..

In the second example, you'll get null if the key doesn't exist. Whether to check or not, depends on your logic.

Try not to waste your time on thinking about performance, containsKey has O(1) time complexity:

This implementation provides constant-time performance for the basic operations (get and put)

4

is it useful to first do a Map.containsKey() check before trying to remove the value from the map?

No, it is counterproductive:

  • In the case when the item is not there, you would see no difference
  • In the case when the item is there, you would end up with two look-ups.

If you want to remove the item unconditionally, simply call map.remove(key).

Same question goes for retrieving values

Same logic applies here. Of course you need to check the result for null, so in this case if stays there.

Note that this cleanup exercise is about readability first, and only then about performance. Accessing a map is a fast operation, so accessing it twice is unlikely to cause major performance issues except for some rather extreme cases. However, removing an extra conditional will make your code more readable, which is very important.

0

The Java documentation on remove() states that it will remove the element only if the map contains such element. So the contains() check before remove() is redundant.

0

This is subjective (and entirely a case of style), but for the case where you're retrieving a value, I prefer the contains(key) call to the null check. Boolean comparisons just feel better than null comparisons. I'd probably feel differently if Map<K,V>.get(key) returned Optional<V>.

Also, it's worth noting the "given no null keys" assertion is one that can be fairly hard to prove, depending on the type of the Map (which you might not even know). In general I think the redundant check on retrieval is (or maybe just feels) safer, just in case there's a mistake somewhere else (knocks on wood, checks for black cats, and avoids a ladder on the way out).

For the removal operation you're spot on. The check is useless.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.