MapのkeySetを回していたら、entrySetの方が適切かを考えてみよう

実に恥ずかしいお話です・・・
つぎのようなコードを書いていたらfindbugsから指摘を受けました。

Map<String,String> map = ....;
for(String key: map.keySet()) {
    String value = map.get(key);
    Hoge hoge = new Hoge(key, value);  
　　hoge.save();　  
}

findbugs曰く、

WMI_WRONG_MAP_ITERATOR

このメソッドは、keySet イテレータから取り出されたキーを使用して、マップエントリの値にアクセスしています。 Map の entrySet イテレータを使用した方が Map.get(key) ルックアップを回避するのでより効率的です。

つまり、「keyとvalueの両方使うのなら、entrySetでfor文を回しなさい」ということ...

Map<String,String> map = ....;
for(Entry<String,String> entry: map.entrySet()) {
    Hoge hoge = new Hoge(entry.getKey(), entry.getValue())
    hoge.save()
}

上のようにしたら怒られなくなりましたw
（この場合、Hogeのコンストラクタの引数をEntryにする方がよいでしょうけど）

まとめ

MapのkeySetを回して処理しているときに、map#getでvalueを取得しているなら、entrySetを回した方が効率も良いし、読みやすい、ということでした。なんかいけてないと思っていったのがようやく解消できました。。。