JavaAntiPatterns

Collection of bad coding practices

Accessing the Map values using keySet iterator

with 12 comments

A common mistake is to retrieve values from a Map while iterating over the Map keys with keySet(). Calling Map.get(key) for each entry is expensive and should be avoided for better performance.

Use entrySet() iterator to avoid the Map.get(key) lookup.

Bad:

for (Object key: map.keySet())
    doSomething(map.get(key));  

Good:

for (Map.Entry entry: map.entrySet())     
    doSomething(entry.getValue());
About these ads

Written by Alex

November 22, 2007 at 12:02 pm

Posted in Collections

Tagged with , , ,

12 Responses

Subscribe to comments with RSS.

  1. And if I need both the key and the value in the body of the loop? Then this is not an anti-pattern, I’d guess.

    Joerg Bullmann

    April 7, 2008 at 3:03 pm

  2. Please disregard — I was writing before thinking.

    Joerg Bullmann

    April 7, 2008 at 3:11 pm

  3. The good method works only when using generics.
    The bad method also works without generics.

    Graeme Byers

    September 28, 2008 at 4:59 pm

  4. I did a benchmark on this , but the ‘bad’ method actually performed better.

    I generated a HashMap with 1000 entries. First i print them using the bad method. After that using the new method.

    Results:
    Bad method took [32] ms
    New method took [47] ms

    Can you elaborate?

    S. Hendriks

    December 10, 2009 at 10:22 am

  5. Never mind my previous post.

    I had a bug in my benchmark program. The results now:

    First try:
    Bad method took [63] ms
    New method took [15] ms

    Second:
    Bad method took [47] ms
    New method took [16] ms

    It really has a great difference

    S. Hendriks

    December 10, 2009 at 10:25 am

    • Yes, every time HashMap.get() is called, it performs a lookup for a map entry with the specified key.

      entrySet() provides a direct access to the internal map data structure (an array of key-value pairs), so no additional lookups are performed – you just iterate over map data as it is.

      I have no idea why it was stated that it works only with generics (in the comment above). Both methods work with or without generics, don’t see any problem there.

      Alex

      December 10, 2009 at 11:39 am

      • It totally makes sense.

        In my case, a map with is now iterated using map.Entry like:

        for (Map.Entry entry: test.entrySet()) {
        // …
        }

        When i do not use it does work, but it gives a warning : “Map.Entry is a raw type. References to generic type Map.Entry should be parameterized”

        I don’t know how this would work in Java 1.4, but at this point I would not care either :)

        stefanhendriks

        December 11, 2009 at 7:46 am

      • For some reason it left out the <String, String> in my previous post…

        stefanhendriks

        December 11, 2009 at 7:47 am

  6. for (Map.Entry entry : map.entrySet()) {
    System.out.println(entry.getKey());
    System.out.println(entry.getValue());
    }

    Picky

    July 19, 2011 at 3:48 pm

  7. wordpress…

    [...]Accessing the Map values using keySet iterator « Java AntiPatterns[...]…

    wordpress

    December 7, 2011 at 6:27 pm

  8. [...] Fuente: http://javaantipatterns.wordpress.com/2007/11/22/accessing-the-map-values-using-keyset-iterator/ Por el momento no hay artículos relacionados. [...]


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: