はじめに

リーダブルコードを読むと変数などの命名に関してはこれほどかと思うほど書いてあると思うが、今回は「統一感のある変数名をつけないとバグらせちゃうよね」パターンを見つけたので参考にしてほしい。

今回書くコードの仕様

userId を入力すると name を返すメソッドを実装したい
まずは（Redisなどの）キャッシュから取ってこようとする
キャッシュに存在しない場合はAPIから取ってこようとする
APIから結果を取得した場合はキャッシュにそれを保存する

僕が見たコード

オレオレJavaっぽいコードで書く
どうでもいいところの実装は省くので想像で補ってほしい

class UserRepository {
  public String getUserNameFromId(int userId) {
    // キャッシュから取得しようと試みる
    String name = getFromCache(userId);

    // キャッシュに存在した場合はその名前を返す
    if(name != null) {
      return name;
    }

    // キャッシュに無い場合はAPI経由で取得する
    String nameFromApi = getFromApi(userId);
    
    // キャッシュに保存する
    setCache(userId, name);

    return name;
  }
}

このコードの問題点

APIから取得してきた結果は nameFromApi という変数名になっているが、キャッシュから取得してきた結果は name という変数名になっている。
それ故に、最後にキャッシュに保存する部分で、 nameFromApi を保存するべきなのに name を返して閉まっている
ついでに name を返してしまっている

改善する

name, nameFromApi という名前に統一感がなく、なんとなくで name を return してしまい間違えてしまった
名前を統一する

class UserRepository {
  public String getUserNameFromId(int userId) {
    // キャッシュから取得しようと試みる
    String nameFromCache = getFromCache(userId);

    // キャッシュに存在した場合はその名前を返す
    if(nameFromCache != null) {
      return nameFromCache;
    }

    // キャッシュに無い場合はAPI経由で取得する
    String nameFromApi = getFromApi(userId);
    
    // キャッシュに保存する
    setCache(userId, nameFromApi);

    return nameFromApi;
  }
}

一時的につけた変数名をそのまま使っちゃったんだ...

僕の経験ですが、「後で精巧しよう」と一時的につけた変数名は、結構な確率でそのままプルリクに出てしまいます。コードを書いた後にテストしたり、他の実装をしているうちに忘れてしまうからです。
さらに今回のケースの場合、パッと見たときに自然なコードに見えてしまい、レビューをすり抜ける確率も高いので危険です。
命名は、デバッグが終わったら消す予定のものでない限り、変数や関数を書く時には決まっているべきです。必要になってから考えるというのは重要なことですが、適当な変数名を適当に書いておくのは、「必要になったのに考えてない」ということになります。
「そもそもその変数が本当に必要なのかわかっていない」「何も意識せずに書いていたら統一的で無い変数名になっていた」は実は危険です。今から自分が書こうとしているコードの全容を全く意識せずに把握出来ていないままコードを書いてしまっています。自分が今からかくコードの立ち位置をはっきりさせてください。