Solved Iterating over a hashmap

Discussion in 'Plugin Development' started by adam753, Dec 3, 2012.

Thread Status:
Not open for further replies.
  1. Offline

    adam753

    Welcome to my problem-du-jour. I've been looking for a way to iterate over a HashMap object for each entry in it. I googled around a little bit and found the code to do so, which uses an Iterator object and Map.Entry<> objects:
    Code:
                    Iterator<Entry<C1, C2>> it = mymap.entrySet().iterator();
                    while (it.hasNext()) {
                        Map.Entry<C1, C2> pairs = (Map.Entry<C1, C2>)it.next();
                        if(pairs.getValue().equals(wrapper)){
                            evt.getPlayer().sendMessage("§4Combination already in use");
                            return;
                        }
                        it.remove();
                    }
    
    That's the code to iterate over a HashMap and exit the function if it contains the value "wrapper" (Note that since both data types in the HashMap are my own classes, I've replaced their names with C1 and C2 for simplicity)
    Now this does appear to work, almost. I can't see anything wrong with it. The problem is, it.remove() seems to actually remove the entry from the map, which of course I don't want. However, if I comment that line out, then I start getting ConcurrentModificationExceptions, to which the solution is to put that line in.
    My question, put simply: what do I do?

    So thanks for hopefully reading all of this, and I hope the answer is nice and simple.
    PS. no, I can't use map.containsValue(), sorry.
     
  2. Offline

    leiger

    The problem is not with commenting out the it.remove() line, but where you are accessing the HashMap from (you aren't using threads safely). That's why you're getting a ConcurrentModificationException.

    Where are you running this code from?
     
  3. Offline

    adam753

    The quoted code is in a function in my listener (onPlayerInteract event), while the map is in my main plugin class. I'm not using threads in my code.

    I also think I should point out that I only get the exceptions when that line isn't in there, but the code doesn't work correctly when it is.
     
  4. Offline

    Ranzdo

    The code that you have written will work (using the Itreator.remove() method is safe from ConcurrentModificationException). However, as you said, using Itreator.remove() does not fit your needs, since the Set reflects the Map (values that is modified in the set is also changed in the Map).

    Could you tell us the line that the ConcurrentModificationException is thrown from (the stack trace) and supply the whole class, I suspect the error lies elsewhere since if removing the elements from the Map makes it work it does not make any sense.
     
  5. Offline

    fireblast709

    Code:java
    1. for(Entry<C1, C2> pairs : mymap.entrySet())
    2. {
    3. if(pairs.getValue().equals(wrapper))
    4. {
    5. evt.getPlayer().sendMessage("§4Combination already in use");
    6. return;
    7. }
    8. }
     
  6. Offline

    adam753

    There doesn't seem to be a line number...
    Code:
    Caused by: java.util.ConcurrentModificationException
            at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
            at java.util.HashMap$EntryIterator.next(Unknown Source)
            at java.util.HashMap$EntryIterator.next(Unknown Source)
            at com.google.common.collect.AbstractBiMap$EntrySet$1.next(AbstractBiMap
    .java:314)
            at com.google.common.collect.AbstractBiMap$EntrySet$1.next(AbstractBiMap
    .java:306)
            at uk.co.tiscali.adam_j_brown.ArcClickListener.onClick(ArcClickListener.
    java:294)
            at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
            at java.lang.reflect.Method.invoke(Unknown Source)
            at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.ja
    va:339)
            ... 16 more
    

    Thank you, that looks a lot simpler.
     
  7. Offline

    Ranzdo

    Code:
    uk.co.tiscali.adam_j_brown.ArcClickListener.onClick(ArcClickListener.
    java:294)
    What is it at line 294 in the ArcClickListener class?
     
  8. Offline

    adam753

    Set this to solved as the forEach loop worked perfectly.

    I would love to explore the original problem with you, but unfortunately I've changed the code now and have no way of getting back and finding out what line 294 was. Sorry.
     
  9. Offline

    fireblast709

    Ranzdo the original problem probably was that he removed items from the iterator, while iterating over it (which in essential is the ConcurrentModificationException
    Code:java
    1. Iterator<Entry<C1, C2>> it = mymap.entrySet().iterator();
    2. while (it.hasNext()) // Looping over it
    3. {
    4. Map.Entry<C1, C2> pairs = (Map.Entry<C1, C2>)it.next();
    5. if(pairs.getValue().equals(wrapper)){
    6. evt.getPlayer().sendMessage("§4Combination already in use");
    7. return;
    8. }
    9. it.remove(); // while removing items from it
    10. }
     
  10. Offline

    Ranzdo

    Except that code you posted will work perfectly fine. Itrator.remove() safely removes it from the list. If it was Set.remove() i would have agreed, but it is not.


    adam753 did also say that the problem was that when he commented OUT the Itrator.remove() method it started throw ConcurrentModificationException which means it had nothing to to with removing elements from the set.

    However, I am glad that the foreach loop worked, even though it still does not make any sense. But that why you do not like ConcurrentModificationException, they are always sneaky and hard to find the cause to ;).
     
Thread Status:
Not open for further replies.

Share This Page