複雑な if-else を解消するちょっとした心がけ

背景

コーディング初心者のコードレビューをしていると、しばしば if-else の深いネストが散見されます。SonarQube などのメトリクスツールでも「cyclomatic complexity が高すぎる」と怒られるやつです。

条件分岐を徹底的に減らしていこうとすると Strategy パターンやら State パターンといったデザインパターンの話がよく出てきますが、そこまでいかずとも、ちょっとした心がけでコードが見やすくなるよ、というお話です。初心者向けです。

例

以下は商品が購入可能かどうかを判定するコードです。

public boolean canPurchase() {
    boolean result;

    // ログインしているか
    if (isLoggedIn()) {
        // 在庫があるか
        if (existsStock()) {
            // 代引きの場合
            if (isPaymentMethodCash()) {
                result = true;
            // クレジット払いの場合
            } else {
                // クレジットカードが登録されているか
                if (isCreditCardRegistered()) {
                    result = true;
                } else {
                    result = false;
                }
            }
        } else {
            result = false;
        }
    } else {
        result = false;
    }

    return result;
}

典型的な、ネストが深くなってしまった例で、非常に処理を追いにくいコードになっています。不慣れな人のコードを見ていると以下のように、ついつい正常系のワークフローを意識してしまっている気がします。

if (1_正常系) {
    if (2_正常系) {
        ...
        ...
        ... // 長い処理
        ...
        ...
    } else {
        // 2_異常系
        return null;
    }
} else {
    // 1_異常系
    throw new SomeException();
}

しかし、ログインしていない、在庫がないといった「特殊な」ケースにおいては、他の条件（支払い方法やカード登録の有無）はどうでも良く、無条件で返却値が決まります（あるいは、例外が発生します）。よってこれらのケースは、他のロジックから分離してあげましょう。

まずは、ログイン条件を分離してあげます。

public boolean canPurchase() {

    // ログインしていなければ無条件で false
    if (!isLoggedIn()) {
        return false;
    }

    // 在庫があるか
    if (existsStock()) {
        // 代引きの場合
        if (isPaymentMethodCash()) {
            return true;
        // クレジット払いの場合
        } else {
            // クレジットカードが登録されているか
            if (isCreditCardRegistered()) {
                return true;
            } else {
                return false;
            }
        }
    } else {
        return false;
    }
}

最初のコードではローカル変数 result に真偽値を詰めて最後に return result していましたが、今回は直ちに真偽値を return しています。このように一時変数を使わず、返り値が確定した時点で返却する方法を早期リターンと呼ぶようです。
また上記のコードのように、冒頭で特殊ケースの早期リターンを行い、以降はその特殊ケースを対象外とする方法をガード節と呼びます。こうすれば else 句を使わずに済み、ネストが解消されます。

在庫条件も分離してみましょう。

public boolean canPurchase() {

    // ログインしてなければ無条件で false
    if (!isLoggedIn()) {
        return false;
    }

    // 在庫がなければ無条件で false
    if (!existsStock()) {
        return false;
    }

    // 代引きの場合
    if (isPaymentMethodCash()) {
        return true;
    // クレジット払いの場合
    } else {
        // クレジットカードが登録されているか
        if (isCreditCardRegistered()) {
            return true;
        } else {
            return false;
        }
    }
}

ネストがだいぶ解消され、処理を追いやすくなったと思います。ちょっとした労力で相当コードの見通しが良くなるコスパのよい方法なので、ぜひ心がけてみてください。

※ ガード節の適用範囲
ここでさらに踏み込むと、代引きの場合は必ず true になっているので、ここにもガード節を適用するとさらにネストを減らせます。

public boolean canPurchase() {

    // ログインしてなければ無条件で false
    if (!isLoggedIn()) {
        return false;
    }

    // 在庫がなければ無条件で false
    if (!existsStock()) {
        return false;
    }

    // 代引きの場合無条件で true
    if (isPaymentMethodCash()) {
        return true;
    }

    // クレジットカードが登録されているか
    if (isCreditCardRegistered()) {
        return true;
    } else {
        return false;
    }
}

ただ、これは好みや方針の問題かもしれませんが、個人的にはやらない方が良いと思います。理由としては、ガード節はあくまで特殊な例をあらかじめ弾くものであり、代引きかクレジットカード払いか、はどちらも正常ケースにおける一般的な条件だからです。対等な条件であれば、if-else で並列に並べるべきだと思います。また以下のように、ガード節の順番が変わると問題が生じてしまいます。

/** まずい例。ログインしてなくても代引きなら true が返る **/
public boolean canPurchase() {

    // 代引きの場合無条件で true
    if (isPaymentMethodCash()) {
        return true;
    }

    // ログインしてなければ無条件で false
    if (!isLoggedIn()) {
        return false;
    }

    // 在庫がなければ無条件で false
    if (!existsStock()) {
        return false;
    }

    // クレジットカードが登録されているか
    if (isCreditCardRegistered()) {
        return true;
    } else {
        return false;
    }
}

ガード節は独立して（順番に依存せず）判定できる範囲に留めておくべきかな、と思っています。