メソッドの冒頭に if 文で return させる場合に else をつけたほうがいい理由

Java8

「メソッドの冒頭で if 文で return させる」っていうのはどういうことかというと

    private static String getContents(HashMap<String, String> myMap, String key) {
        if (myMap == null) {
            return "";
        }
        return myMap.get(key);
    }

みたいなヤツのことです。

この場合だと、受け取った Map がnullだったときに NullPointerException になるのを防ぐために、メソッドの冒頭でnullチェックを行っています。 で、もしnullだったら空文字を返しておしまいなんですが、問題はこのあとですね。

選択肢は２つ。 ひとつは上記で書いたこれ

    private static String getContents(HashMap<String, String> myMap, String key) {
        if (myMap == null) {
            return "";
        }
        return myMap.get(key);
    }

もうひとつは、elseを付けるバージョン。

    private static String getContents(HashMap<String, String> myMap, String key) {
        if (myMap == null) {
            return "";
        } else {
            return myMap.get(key);
        }
    }

処理としてはどちらも同じです。 単に見た目の問題なんですが、違いが大きく出てくるのは以下の場合です。

if文以降(本処理)が長くなる場合

本処理が長くなる場合は、else をつけないほうがスッキリします。

  • else あり
    private static String getContents(HashMap<String, String> myMap, String key) {
        if (myMap == null) {
            return "";
        } else {
            String result = myMap.get(key);
            .
            .
            .
            .
            .
            .
            .
            .
            return result;
        }
    }
  • else なし
    private static String getContents(HashMap<String, String> myMap, String key) {
        if (myMap == null) {
            return "";
        }
        String result = myMap.get(key);
        .
        .
        .
        .
        .
        .
        .
        .
        return result;
    }

なので、個人的には else つけないほうが好きなんですが、 違う観点もあります。

「ここで else をつけなくても良いという判断は、if文の処理に依存している」

というものです。

つまり、ここで「elseなくてもいいじゃん」って思ったのは、 if の中で return してるからです。

より正確に言えば、if の中で return してることをちゃんと把握できているから、です。

もしここで return しないのであれば else は必要になるかもしれません。

例えば「mapがnullだったらデフォルトの値を利用して後続で整形して返す」という仕様変更があった場合、elseがあるとないとでは全然変わってきます。

  • else あり
    private static String getContents(HashMap<String, String> myMap, String key) {
        String value;
        if (myMap == null) {
            value = DEFAULT_VALUE;
        } else {
            value = myMap.get(key);
        }
        return optimizeFormat(value);
    }

ここで else がなければ、mapがnullの場合、当然NPEで落ちます。

  • else なし
    private static String getContents(HashMap<String, String> myMap, String key) {
        String value;
        if (myMap == null) {
            value = DEFAULT_VALUE;
        }
        value = myMap.get(key);
        return optimizeFormat(value);
    }

ちなみに最初の else 無しバージョンはこれ

    private static String getContents(HashMap<String, String> myMap, String key) {
        if (myMap == null) {
            return "";
        }
        return myMap.get(key);
    }

似てますね。

これが上記の挙動のおかしなメソッドに変貌するのはあっという間だと思います。

else をつけないほうがインデントが減ってぱっとみシンプルでいいんですが、のちのちの安易な修正によりバグを埋め込む可能性が若干残ってしまうんですね。

結論

else をつけるかつけないかの2択でいうと、場合にもよりますが「つけたほうが安全」ということになります。

ただ、、そもそもロジックの途中で return させることがあまり良くないですね、やっぱり。