Map.keySet()で取得したキーのセットをそのままローカル変数に入れるな！　Map.keySet()は元のマップと関連付けられていることを意識せよ！

Java 格言

よくコーディングでjava.util.Mapを使用する。そのキーのセットを取得するにはMap.keySet()メソッドを使用することになる。
このメソッドで得られたSetは、元のMapのキーのビューになっている。つまり、Mapの内容に関連付けられているわけだ。

このSetを取得してからMapの内容を変更すると（Map.put(K, V)で追加、Map.remove(K)で削除、など）、そのセットの内容もそれに合わせて増減する。それを意識しないで使うと思わぬバグの原因になる。

// 例．商品IDと商品情報のマップ
Map<Integer, ItemBean> itemMap = new HashMap<Integer, ItemBean>();
// （１）ここでmapに値を設定
itemMap.put(itemId1, item1);
itemMap.put(itemId2, item2);
// ...

// （２）ここでマップに設定されたキー（例．商品ID）を取得する
Set<Integer> itemIdSet = itemMap.keySet();

// （３）ここでマップに値を追加、削除する
itemMap.put(itemId3, itemBean3);
itemMap.remove(itemId2);
// ....

// （４）ここで（２）の時点のsetの内容で処理を行おうとすると、内容は（３）によって書き換えられている
for (Integer itemId : itemIdSet) {
    // ....
}

また、これらはMap.values()で取得した値のコレクション、Map.entrySet()で取得したエントリーのセットに対しても同様である。

Map.keySet()で取得したセットをそのままローカル変数に代入しない

通常、Mapに関連付けられたセットではなく、Map.keySet()を実行したときのセットとして使用したい場合が多いと思われる。そのような場合はkeySet()で取得したセットをそのまま使用するのではなく、次のように取得と同時に別のセットかリストに変換すべきだ。

// セットを取得する場合
Set<Integer> itemIdSet = new HashSet<Integer>(itemMap.keySet());
// または
Set<Integer> originalItemIdSet = new HashSet<Integer>(itemMap.keySet());

// リストを取得する場合
List<Integer> itemIdList = new ArrayList<Integer>(itemMap.keySet());
// または
List<Integer> originalItemIdList = new ArrayList<Integer>(itemMap.keySet());

これだと元のマップを意識することなく、その時点での内容を保持したセットやリストとして使用できる。特にセットからセットを作る場合はオブジェクトを重複して作成しているようで無駄に感じる人もいるかもしれないが、ほんの少しのオーバーヘッドよりも誰にでもわかりやすい、誤解やバグの原因が少ないコーディングを心がけたいものだ。

Map.keySet()をマップに関連付けられたセットとしてそのまま使用する場合はローカル変数名を工夫する

また、あまり使用することもないと思われるが、マップに関連付けられたセットとして使用する場合には、ローカル変数名に何らかのマークを付けておくべきだ。
Map.keySet()で取得したセットだ、ということが分かれば意識できるのであれば、例えばitemIdKeySetという名前でもいいだろう。