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

New check: PreferMapEntryToIterateWholeMap #77

Closed
romani opened this Issue Nov 3, 2012 · 1 comment

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Nov 3, 2012

Consider user Map.Entry for whole map iteration instead of
getKeys() and then get value by key.
check could simply check while/for for such case. This check will not find all
weird cases but will help significantly with major cases, that are done as C++ habit, refactoring, .....

http://sourceforge.net/p/checkstyle/feature-requests/586/

from Check JavaDoc:
This check can help you to write the whole for-each map iteration more correctly:
If you iterate over a map using map.keySet() or map.entrySet(), but your code uses only map values, Check will propose you to use map.values() instead of map.keySet() or map.entrySet(). Replacing map.keySet() or map.entrySet() with map.values() for such cases can a bit improve an iteration performance.
Bad:

for (Map.Entry; entry : map.entrySet()){
   System.out.println(entry.getValue());
}
for (String key : map.keySet(){
   System.out.println(map.get(key));
}

Good:

for (String value : map.values()){
   System.out.println(value);
}

If you iterate over a map using map.entrySet(), but never call entry.getValue(), Check will propose you to use map.keySet() instead of map.entrySet(). to iterate over map keys only.
Bad:

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

Good:

for (String key : map.keySet()){
   System.out.println(key);
}

If you iterate over a map with map.keySet() and use both keys and values, check will propose you to use map.entrySet() to improve an iteration performance by avoiding search operations inside a map. For this case, iteration can significantly grow up a performance.
Bad:

for (String key : map.keySet()){
   System.out.println(key + "  " + map.get(key));
}

Good:

for (Map.Entry entry : map.entrySet()){
   System.out.println(entry.getValue() + "   " + entry.getKey());
}
@daniilyar

This comment has been minimized.

Show comment
Hide comment
@daniilyar

daniilyar Jan 27, 2013

Member

I think, this is a useful check (idea was taken from the same-named FindBugs Check).

The main check idea is:

  1. To force developer to iterate over maps using Map.Entry instead of [ keyset() + getValue(key) ] for every map entry in case if developer needs both keys and values.
  2. To force developer to use keySet() method in case if developer does not use the appropriate values ( == if map.getValue(key) method is never executed inside a loop).

Such type of refactoring can improve performance when it is done in frequently-used code.

But if developer needs keys only for his algorithm, we should not prevent this, or show any warnings.

Code examples:
http://www.sergiy.ca/how-to-iterate-over-a-map-in-java/

http://stackoverflow.com/questions/12639259/findbugs-warning-inefficient-use-of-keyset-iterator-instead-of-entryset-iterato

and:

        Map<String, String> map = Maps.newLinkedHashMap();
        map.put("key1", "value1");
        map.put("key2", "value2");
        map.put("key3", "value3");

        System.out.println("\nUsing map.keySet():");
        for (String key : map.keySet()) {
            System.out.println(key + " --> " + map.get(key));
        }

        System.out.println("\nUsing Map.Entry:");
        for (Map.Entry<String, String> entry : map.entrySet()) {
            System.out.println(entry.getKey() + " --> " + entry.getValue());
        }
Member

daniilyar commented Jan 27, 2013

I think, this is a useful check (idea was taken from the same-named FindBugs Check).

The main check idea is:

  1. To force developer to iterate over maps using Map.Entry instead of [ keyset() + getValue(key) ] for every map entry in case if developer needs both keys and values.
  2. To force developer to use keySet() method in case if developer does not use the appropriate values ( == if map.getValue(key) method is never executed inside a loop).

Such type of refactoring can improve performance when it is done in frequently-used code.

But if developer needs keys only for his algorithm, we should not prevent this, or show any warnings.

Code examples:
http://www.sergiy.ca/how-to-iterate-over-a-map-in-java/

http://stackoverflow.com/questions/12639259/findbugs-warning-inefficient-use-of-keyset-iterator-instead-of-entryset-iterato

and:

        Map<String, String> map = Maps.newLinkedHashMap();
        map.put("key1", "value1");
        map.put("key2", "value2");
        map.put("key3", "value3");

        System.out.println("\nUsing map.keySet():");
        for (String key : map.keySet()) {
            System.out.println(key + " --> " + map.get(key));
        }

        System.out.println("\nUsing Map.Entry:");
        for (Map.Entry<String, String> entry : map.entrySet()) {
            System.out.println(entry.getKey() + " --> " + entry.getValue());
        }

@ghost ghost assigned maxvetrenko Jan 27, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 3, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 5, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 7, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 7, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 9, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 9, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 9, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 11, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 11, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 11, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 11, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 15, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 23, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Oct 23, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Nov 3, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Nov 6, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Nov 6, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Nov 6, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Dec 8, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Dec 13, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Dec 26, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Dec 26, 2013

Fixed #77. PreferMapEntryToIterateWholeMapCheck was introduced
Fixed #77. MapIterationInForEachLoop was introduced

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Dec 26, 2013

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 8, 2014

Fixed #77. PreferMapEntryToIterateWholeMapCheck was introduced
Fixed #77. MapIterationInForEachLoop was introduced

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 8, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 8, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 8, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 9, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 13, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 14, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 16, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 18, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 20, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 21, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 25, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Jan 27, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Feb 2, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Feb 5, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Feb 5, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Feb 16, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Feb 17, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Feb 26, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Mar 1, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Mar 4, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Mar 4, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Mar 6, 2014

@daniilyar daniilyar closed this in 474a5ef Mar 8, 2014

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