301

I was doing:

for (Object key : map.keySet())
    if (something)
        map.remove(key);

which threw a ConcurrentModificationException, so i changed it to:

for (Object key : new ArrayList<Object>(map.keySet()))
    if (something)
        map.remove(key);

this, and any other procedures that modify the map are in synchronized blocks.

is there a better solution?

rogerdpack
  • 62,887
  • 36
  • 269
  • 388
pstanton
  • 35,033
  • 24
  • 126
  • 168
  • 1
    If this method and the other method that modify the map are in synchronized blocks, I don't see why you have to do anything? Maybe I'm not understanding your question completely? Can ou please post the rest of the code? – Amir Afghani Dec 10 '09 at 23:42
  • @Raedwald, this question and it's accepted answer are more succinct than the other IMO. – pstanton Nov 17 '17 at 00:46

12 Answers12

408

Here is a code sample to use the iterator in a for loop to remove the entry.

Map<String, String> map = new HashMap<String, String>() {
  {
    put("test", "test123");
    put("test2", "test456");
  }
};

for(Iterator<Map.Entry<String, String>> it = map.entrySet().iterator(); it.hasNext(); ) {
    Map.Entry<String, String> entry = it.next();
    if(entry.getKey().equals("test")) {
        it.remove();
    }
}
Paul Rooney
  • 20,879
  • 9
  • 40
  • 61
Gennadiy
  • 17,657
  • 4
  • 26
  • 21
  • 24
    So, you do: `it.remove()` and never `collection.remove(key)` inside a loop. It's great! – David Gras Aug 25 '14 at 14:32
  • 12
    This answer is good for code before Java 8, but elron's answer is preferable if you are using Java 8. http://stackoverflow.com/a/29187813/2077574 – KPD Jul 08 '15 at 18:42
  • 1
    This works great in Android too. – Apostrofix Jan 13 '16 at 13:01
  • 1
    If an item were added to this Map while it was being recursed to remove items wouldn't it still throw a ConcurrentModificationException? – Bill Mote Feb 16 '16 at 18:18
  • Hashmap has a fail-fast iterator. Then why does it.remove() not throw a ConcurrentModificationException ? – sumanth232 Jul 20 '16 at 08:58
  • Since Java 8, you can also use this instead: `map.entrySet().removeIf(entry -> entry.getValue().equals("test"));` – arenaq Mar 22 '18 at 19:22
  • 1
    @KPD This is still better if we have some other logic as well inside the for loop, which was my case. – Optimus Prime Sep 24 '19 at 05:14
353

As of Java 8 you could do this as follows:

map.entrySet().removeIf(e -> <boolean expression>);

Oracle Docs: entrySet()

The set is backed by the map, so changes to the map are reflected in the set, and vice-versa

Klesun
  • 12,280
  • 5
  • 59
  • 52
koders
  • 5,654
  • 1
  • 25
  • 20
110

Use a real iterator.

Iterator<Object> it = map.keySet().iterator();

while (it.hasNext())
{
  it.next();
  if (something)
    it.remove();
 }

Actually, you might need to iterate over the entrySet() instead of the keySet() to make that work.

Paul Tomblin
  • 179,021
  • 58
  • 319
  • 408
  • 3
    This does seem a bit more elegant than my solution of iterating over the entry set. Its not very obvious though that removing from the key set removes things from the map (i.e. the key set can be a copy) – Gennadiy Dec 10 '09 at 23:43
  • 11
    Sorry to double comment. I have confirmed that removing from the key set indeed removes from the map, although not as obvious as removing from the entry set. – Gennadiy Dec 10 '09 at 23:44
  • 1
    Iterating over the key set is the way to do it. – Tom Hawtin - tackline Dec 11 '09 at 01:17
  • (Although to match the question, it should be `Iterator`.) – Tom Hawtin - tackline Dec 11 '09 at 01:18
  • Tom, in the first loop he's using "for (Object key:", so that's why I used an Object iterator. – Paul Tomblin Dec 11 '09 at 01:31
  • 8
    `Iterator.next()` or in this case `it.next()` must be invoked before the call to `remove` otherwise there will be an `IllegalStateException`. – H2ONaCl Oct 02 '12 at 05:14
  • @broiyan, you are correct, I forgot that. – Paul Tomblin Oct 02 '12 at 05:26
  • 7
    For the record, this also works for the map values: `Iterator it = map.values().iterator()`, then `it.remove()` will remove it entry. – olafure Jun 30 '15 at 13:56
  • Hashmap has a fail-fast iterator. Then why does it.remove() not throw a ConcurrentModificationException ? – sumanth232 Jul 20 '16 at 08:58
  • @sumanth232 A fail-fast iterator fails when _something else_ modifies the container concurrently. You are allowed to use the iterator as intended. In this answer, nothing is modifying the key Set except for the iterator, and that is translated through to the Map owning that Set as well. Only one operation per container is occurring at a time; no concurrency. – Matthew Read Mar 05 '19 at 19:36
50

is there a better solution?

Well, there is, definitely, a better way to do so in a single statement, but that depends on the condition based on which elements are removed.

For eg: remove all those elements where value is test, then use below:

map.values().removeAll(Collections.singleton("test"));

UPDATE It can be done in a single line using Lambda expression in Java 8.

map.entrySet().removeIf(e-> <boolean expression> );

I know this question is way too old, but there isn't any harm in updating the better way to do the things :)

Shishir Kumar
  • 7,981
  • 3
  • 29
  • 45
  • 1
    It may be an old question but that really helped me. I'd never heard of Collections.singleton() and I had no idea you could remove elements from a map by calling removeAll() on values()! Thank you. – Paul Boddington Sep 01 '14 at 01:32
  • 2
    To remove the element which key is test: map.keySet().removeAll(Collections.singleton("test")); – Marouane Lakhal Aug 31 '16 at 15:11
  • 1
    @MarouaneLakhal To remove the element which key is test, wouldn't you just do map.remove("test"); ? – dzeikei Feb 23 '17 at 23:53
  • 1
    @dzeikei as stated in the question, map.remove(key) threw a ConcurrentModificationException when he was looping over the map.keySet(). – Marouane Lakhal Feb 24 '17 at 11:25
  • 1
    @MarouaneLakhal If you already know the key of an element of the map you want to remove already, why are you looping in the first place? There can only be one entry in the map with the same key. – dzeikei Feb 28 '17 at 10:43
25

ConcurrentHashMap

You can use java.util.concurrent.ConcurrentHashMap.

It implements ConcurrentMap (which extends the Map interface).

E.g.:

Map<Object, Content> map = new ConcurrentHashMap<Object, Content>();

for (Object key : map.keySet()) {
    if (something) {
        map.remove(key);
    }
}

This approach leaves your code untouched. Only the map type differs.

ROMANIA_engineer
  • 54,432
  • 29
  • 203
  • 199
  • 9
    The problem with this approach is that a ConcurrentHashMap does not allow "null" as value (or key), so if by any chance you are dealing with a map containing a null, you could not use this approach. – fiacobelli Feb 02 '15 at 18:21
9

Java 8 support a more declarative approach to iteration, in that we specify the result we want rather than how to compute it. Benefits of the new approach are that it can be more readable, less error prone.

public static void mapRemove() {

    Map<Integer, String> map = new HashMap<Integer, String>() {
        {
            put(1, "one");
            put(2, "two");
            put(3, "three");
        }
    };

    map.forEach( (key, value) -> { 
        System.out.println( "Key: " + key + "\t" + " Value: " + value );  
    }); 

    map.keySet().removeIf(e->(e>2)); // <-- remove here

    System.out.println("After removing element");

    map.forEach( (key, value) -> { 
        System.out.println( "Key: " + key + "\t" + " Value: " + value ); 
    });
}

And result is as follows:

Key: 1   Value: one
Key: 2   Value: two
Key: 3   Value: three
After removing element
Key: 1   Value: one
Key: 2   Value: two
Paul Rooney
  • 20,879
  • 9
  • 40
  • 61
Vineet kaushik
  • 351
  • 3
  • 4
5

You have to use Iterator to safely remove element while traversing a map.

Alexander Pogrebnyak
  • 44,836
  • 10
  • 105
  • 121
4

I agree with Paul Tomblin. I usually use the keyset's iterator, and then base my condition off the value for that key:

Iterator<Integer> it = map.keySet().iterator();
while(it.hasNext()) {
    Integer key = it.next();
    Object val = map.get(key);
    if (val.shouldBeRemoved()) {
        it.remove();
    }
}
goatlinks
  • 761
  • 1
  • 4
  • 10
  • 10
    You should use the entrySet instead of using the keySet and doing a get every time. FindBugs even has a detector for this: http://findbugs.sourceforge.net/bugDescriptions.html#WMI_WRONG_MAP_ITERATOR – Tim Büthe Apr 27 '12 at 11:46
2

An alternative, more verbose way

List<SomeObject> toRemove = new ArrayList<SomeObject>();
for (SomeObject key: map.keySet()) {
    if (something) {
        toRemove.add(key);
    }
}

for (SomeObject key: toRemove) {
    map.remove(key);
}
vickirk
  • 3,979
  • 2
  • 22
  • 37
2

And this should work as well..

ConcurrentMap<Integer, String> running = ... create and populate map

Set<Entry<Integer, String>> set = running.entrySet();    

for (Entry<Integer, String> entry : set)
{ 
  if (entry.getKey()>600000)
  {
    set.remove(entry.getKey());    
  }
}
Arnab Das
  • 113
  • 1
  • 5
Aftershock
  • 5,205
  • 4
  • 51
  • 64
1

Maybe you can iterate over the map looking for the keys to remove and storing them in a separate collection. Then remove the collection of keys from the map. Modifying the map while iterating is usually frowned upon. This idea may be suspect if the map is very large.

Vincent Ramdhanie
  • 102,349
  • 23
  • 137
  • 192
-2
Set s=map.entrySet();
Iterator iter = s.iterator();

while (iter.hasNext()) {
    Map.Entry entry =(Map.Entry)iter.next();

    if("value you need to remove".equals(entry.getKey())) {
         map.remove();
    }
}
Paul Rooney
  • 20,879
  • 9
  • 40
  • 61