その条件、同時に比較する必要ある?

1つのif文に、大量の条件がandやorで結合されているソースをたまに見ます。

確かに、1つにまとめることで処理の重複を防ぐことができ、また、プログラムの行数も削減できます。

しかし、あえて複数のif文に分けた方が可読性や保守性が向上するケースが多々あります。

 

例えば、このようなメソッドがあるとします。

public int getPrice(int price) {
    // 特別会員、またはクーポンを持っている一般会員の場合、割引価格
    if(this.isSpecialMember() || (this.isNormalMember() && this.hasCoupon())){
        return price * 0.8;
    }
    // それ以外の場合、定価通り
    return price;
}

 

このメソッドは下記のように書いた方が、行数は増えますが可読性や保守性が向上します。

public int getPrice(int price) {
    // 特別会員の場合、割引価格
    if(this.isSpecialMember()) {
        return price * 0.8;
    }
    // クーポンを持っている一般会員の場合、割引価格
    if(this.isNormalMember() && this.hasCoupon()) {
        return price * 0.8;
    }
    // それ以外の場合、定価通り
    return price;
}

こうすると、「特別会員である」と「クーポンを持っている一般会員である」がそれぞれ独立したひと固まりの条件であり、同時に起こり得ないということが後から見ても即座に分かります。

また、この2つで割引率が別々になった場合の修正も容易になります。

 

こんな書き方したらif文が大量に並んで読みにくくなるよ・・・という場合は、そもそもの条件(仕様)に問題がある可能性があります。

 

更に、ここまでやるかは処理の量や修正頻度などに応じてケースバイケースとなりますが、処理の重複部分をメソッドとして切り出せば最も理想的な形になります。

public int getPrice(int price) {
    // 特別会員の場合、割引価格
    if(this.isSpecialMember()) {
        return this.getSpecialPrice(price);
    }
    // クーポンを持っている一般会員の場合、割引価格
    if(this.isNormalMember() && this.hasCoupon()) {
        return this.getSpecialPrice(price);
    }
    // それ以外の場合、定価通り
    return price;
}

private int getSpecialPrice(int price) {
    return price * 0.8;
}

話がそれてしまいますが、1行しかないメソッドを作ることに抵抗を感じる必要はありません。

その処理が部品として共通化する価値があるのなら、たとえ1行でも積極的にメソッドとして切り分けるべきです。

 

もう1つ例を挙げます。

こちらのサンプルソースを見てください。

// ログイン中、かつ、権限が管理者かリーダーの場合
if(this.isLogin() && (this.auth == AUTH_ADMIN || this.auth == AUTH_LEADER)) {
    (略)
}

これもよくある形ですが、決して読みやすいとは言えません。

なぜならば、「ログイン済みかどうか」ということと「ログインしているユーザーがどんな権限を持っているか」ということは同列の比較ではないからです。

 

表現が抽象的になってしまいましたが、例えると「野菜である」ことと「人参、または、玉ねぎである」ことを同時にチェックしているようなものです。

ここは、あえてif文を入れ子にすることで可読性が向上します。

// ログイン中の場合
if(this.isLogin()) {
    // 権限が管理者かリーダーの場合
    if(this.auth == AUTH_ADMIN || this.auth == AUTH_LEADER) {
        (略)
    }
}

ネストが深くなってしまいますが、それでも最初のソースよりは読みやすいです。

 

更に、ネストを浅くする手法として『深いネストはバグの温床』に書いたような手法が使える可能性がありますが、それは次のステップです。

実際にその手法が使えるかは前後の処理次第ですが、一応サンプルとして挙げておきます。

// ログイン中ではない場合、処理終了
if(!this.isLogin()) {
    return;
}
// 権限が管理者かリーダーの場合
if(this.auth == AUTH_ADMIN || this.auth == AUTH_LEADER) {
    (略)
}

 

このように、複数のif文に分けたり入れ子にするという、通常のリファクタリングに逆行するような書き方が結果として可読性向上につながることもあります。

if文は極力1つにまとめるのが正解だと思っている方は、本当にそれが適切か見直してみてはいかがでしょうか。




コメントを残す

メールアドレスが公開されることはありません。 * が付いている欄は必須項目です