C#リーダブルコード #07_複数の比較を1回のif文でやらない

「今回は複数の比較を1回でやらない」というテーマで解説していきます。

BAD:複数の比較を1回にやらない
ANDの場合は階層を分ける
ORの場合
- NOTBAD:複数の比較を1回にやらない
- GOOD:メソッド化を検討する
GOOD:ANDでもメソッド化を検討する
ANDとORが混ざっていたらなおさらメソッド化する

BAD:複数の比較を1回にやらない

次のような例を見てください。

private void 複数の比較を1回のif文でやらない(int valueA, int valueB)
{
    //BAD:複数の比較を1回にやらない
    if (valueA == 1 && valueB == 1)
    {
        //処理
    }
}

private void 複数の比較を1回のif文でやらない(int valueA, int valueB)

{

//BAD:複数の比較を1回にやらない

if (valueA == 1 && valueB == 1)

{

//処理

}

valueAとvalueBを「＆＆」でつないで比較しています。極力こういった書き方はしない方がいいと思います。今回の例では2個の比較ですが，3個4個とある場合や，ANDとORが混ざっていたり，小確固で優先される比較を囲ったりしている場合は，どんどんわかりづらくなり，非常にバグの元となります。

ANDの場合は階層を分ける

複数の比較をANDでつないで判定する場合は，階層を分けることで対処できます。

//複数の比較を1回のif文でやらない
private void 複数の比較を1回のif文でやらない(int valueA, int valueB)
{
    //BAD:複数の比較を1回にやらない
    if (valueA == 1 && valueB == 1)
    {
        //処理
    }

    //ANDの場合
    //NOTBAD:階層を分ける
    if (valueA == 1)
    {
        if (valueB == 1)
        {
            //処理
        }
    }
}

//複数の比較を1回のif文でやらない

private void 複数の比較を1回のif文でやらない(int valueA, int valueB)

{

//BAD:複数の比較を1回にやらない

if (valueA == 1 && valueB == 1)

{

//処理

}

//ANDの場合

//NOTBAD:階層を分ける

if (valueA == 1)

{

if (valueB == 1)

{

//処理

}

このように，valueAの判定を通過した場合のみvalueBの判定をするように，if文をネストすれば，if文の階層は深くなりますが，判断は1回ずつしか行わないため，&&で連結するよりもバグりにくいでしょう。

ORの場合

NOTBAD:複数の比較を1回にやらない

2つの判定をOR判定する場合は，先ほどのようにif文をネストしてやるわけにはいきませんね。valueAの判定がFalseでもvalueBの判定は行いたいわけですから，次のように「||」で連結して複数の判定をしなくてはなりません。

//ORの場合
//NOTBAD:複数の比較を1回にやらない
if (valueA == 1 || valueB == 1)
{
    //処理
}

//ORの場合

//NOTBAD:複数の比較を1回にやらない

if (valueA == 1 || valueB == 1)

{

//処理

}

ただ，この場合は，前述の通り，バグの温床になるので，次に紹介するメソッド化を検討するほういいでしょう。

GOOD:メソッド化を検討する

次に例は，IsWriteLosgというメソッドを作成して対応しています。

//複数の比較を1回のif文でやらない
private void 複数の比較を1回のif文でやらない(int valueA, int valueB)
{
    ・・・


    //GOOD:メソッド化を検討する
    if (IsWriteLosg(valueA, valueB))
    {
        //処理
    }

}

private bool IsWriteLosg(int valueA, int valueB)
{
    if (valueA == 1)
    {
        return true;
    }

    if (valueB == 1)
    {
        return true;
    }

    return false;
}

//複数の比較を1回のif文でやらない

private void 複数の比較を1回のif文でやらない(int valueA, int valueB)

{

・・・

//GOOD:メソッド化を検討する

if (IsWriteLosg(valueA, valueB))

{

//処理

}

private bool IsWriteLosg(int valueA, int valueB)

{

if (valueA == 1)

{

return true;

}

if (valueB == 1)

{

return true;

}

return false;

}

IsWriteLosgメソッドの中では，valueAが1ならTrue，valueBが1でもTrueが返却されるようになっており，どちらかが１ならTrueになるため，最初の「valueA == 1 || valueB == 1」と同じ動作をします。動作は同じですが，メソッドの中で，１つずつ判定を行い，どちらかがTrueの場合はすぐにメソッドを抜けることで，１つの判定のみの実施になっており，読みやすさとバグの混入を防ぐ効果があります。