見出し画像

きれいなコードとは?(2)

前回、きれいなコードとは?という記事を書きましたが、第二弾を書こうと思います。

どこで値を返す?

よくあるサンプルで時間にあった挨拶を返すメソッドを考えてみます。

/**
 * 挨拶を返すメソッド.
 * @param hour 時間
 * @return 時間に対応した挨拶
 */
String getGreetingMessage(int hour) {
    String greeting;
    if( hour > 5 && hour < 12 ) {
        greeting = "Good Morning!";
    }
    else if( hour >= 12 && hour < 18 ) {
        greeting = "Good Afternoon!";
    }
    else if( hour >= 18 && hour < 23 ) {
        greeting = "Good Evening!";
    }
    else {
        greeting = "Sorry, We're sleeping.";
    }
    return greeting;
}

ローカル変数で挨拶を宣言して、条件に合ったら代入して最後に値を返すというメソッドです。これは次のように書き換えても同じです。

/**
 * 挨拶を返すメソッド.
 * @param hour 時間
 * @return 時間に対応した挨拶
 */
String getGreetingMessage(int hour) {
    if( hour > 5 && hour < 12 ) {
        return "Good Morning!";
    }
    else if( hour >= 12 && hour < 18 ) {
        return "Good Afternoon!";
    }
    else if( hour >= 18 && hour < 23 ) {
        return "Good Evening!";
    }
    else {
        return "Sorry, We're sleeping.";
    }
}

条件に入ったらすぐに値を返すように書き換えることで、ローカル変数のgreetingは不要になりました。こちらの方が続きの処理がないことがわかるので見やすいですね。もちろん、これも状況によるので必ずしも良いコードとは言えませんが、シンプルになったといえると思います。
もっとシンプルにするのなら、全てのelseを取り除くことも考えられますね。この例の場合、それぞれの条件に入ったら必ず値を返しているので、elseはなくても問題ありません。

見やすい条件式とは?

例えば「15歳以上でないと見てはいけない映画」という条件を表すとどうなるでしょうか?
次のようなメソッドを考えてみました。

/**
 * 映画の視聴可能年齢チェック.
 * @param age 年齢
 * @return true:視聴可能, false:視聴不可
 */
boolean canWatchMovie(int age) {
    if( !( age >= 15 ) ) {
        return false;
    } else {
        return true;
    }
}

確かに「15歳以上でないと見れない」という条件は満たしていますが、何か直感的に理解しづらいですよね。こういう時は条件を否定文ではなく肯定文に書き換えると分かりやすく書けます。

書き換えるとこうなります。

「15歳以上は視聴可能です」

これなら簡単に書けますね。

/**
 * 映画の視聴可能年齢チェック.
 * @param age 年齢
 * @return true:視聴可能, false:視聴不可
 */
boolean canWatchMovie(int age) {
    if( age >= 15 ) {
        return true;
    } else {
        return false;
    }
}

なぜ1つ目の例は読みにくいのか、フローチャートを思い浮かべると分かりやすいと思います。

フローチャートでは条件式の真下に降りる方向(下図では処理1へと向かう方向)がtrue(yes)の処理で、横に向かう方向(下図では処理2へと向かう方向)がfalse(no)の処理と読み取るのが一般的です。

画像1

1つ目の例の場合は、条件式にあった場合falseとなり、外れた場合trueを返すことになっています。また、条件式の中身が否定形になっています(!演算子で打ち消されています)。条件式の中身は肯定文である方が人間は直感的に読めます。そして、!演算子は見落とすこともあるので、誤解を招きやすいです。肯定文で書けるなら肯定文で書いた方が見やすくなるケースの方が多いです。わからなくなったらフローチャートに書きやすい条件式にするという解決策もありますね。

複雑な条件式にどう対応するか?

複雑な条件式としてパッと思いつくのは「うるう年判定」ですね。

うるう年の条件はご存じでしょうか?
4で割れる年だったらすべてうるう年でしょうか?
実はそれだけではないのです。うるう年の条件は以下の通りです。

1.西暦年が4で割り切れる年は(原則として)閏年。
2.ただし、西暦年が100で割り切れる年は(原則として)平年。
3.ただし、西暦年が400で割り切れる年は必ず閏年。
《Wikipedia 閏年 より引用》

また、こう書き換えることもできる。

1.400で割り切れる場合は閏年
2.400で割り切れず、100で割り切れる場合は平年
3.条件 1. と 2. を両方とも満たさない(400で割り切れず、かつ100でも割り切れない)場合、4で割り切れれば閏年、そうでなければ平年
《Wikipedia 閏年 より引用》

意外と複雑な条件があり、400年間に97回閏年が来るようになっています。

さて、これを条件式化することはできるでしょうか?

/**
 * うるう年判定(例1).
 * @param year 年
 * @return true:うるう年, false:平年
 */
 boolean isLeapYear(int year) {
     boolean isLeap;
     if(year % 400 == 0) {
         // 400で割り切れる場合
         isLeap = true;
     } else {
         // 400で割り切れない場合
         if( year % 100 == 0 ) {
             // 100で割り切れる場合
             isLeap = false;
         } else {
             // 400でも100でも割り切れない場合
             if( ( year % 4 == 0 ) {
                 // 4では割り切れる場合
                 isLeap = true;
             } else {
                 // 4でも割り切れない場合
                 isLeap = false;
             }
         }
     }
     return isLeap;
 }

一応書けた気がします。ただ、if節の中にif節が入れ子になっていて見づらいですね。入れ子にならないように条件式を考えましょう。

/**
 * うるう年判定(例2).
 * @param year 年
 * @return true:うるう年, false:平年
 */
 boolean isLeapYear(int year) {
     boolean isLeap;
     if(year % 400 == 0) {
         // 400で割り切れる場合
         isLeap = true;
     } else if( (year % 400 != 0) && (year % 100 == 0) ) {
         // 400で割り切れず、100では割り切れる場合
         isLeap = false;
     } else if( (year % 400 != 0) && (year % 100 != 0) && (year % 4 == 0) ) {
         // 400でも100でも割り切れず、4で割り切れる場合
         isLeap = true;
     } else {
         // 上記以外
         isLeap = false;
     }
     return isLeap;
 }

これは条件式が冗長すぎますよね。そしてこうなると最後のelseに入ったときに漏れた条件がないか心配になりますよね。どうすればよくなるでしょうか?

まず、2つ目以降の条件式に入る時点で400で割れるか否かの条件は判定済みなので、そこに(year % 400 != 0)という条件は不要ですね。同様に3つ目の条件式の(year % 100 != 0)も不要ですね。そして、条件成立したら、すぐに抜けた方がチェックもしやすいですね。というわけで以下のように書き換えができます。

/**
 * うるう年判定(例3).
 * @param year 年
 * @return true:うるう年, false:平年
 */
 boolean isLeapYear(int year) {
     if(year % 400 == 0) {
         // 400で割り切れたらうるう年確定!
         return true;
     } 
     // 以下は400で割り切れない場合
     if(year % 100 == 0) {
         // (400で割り切れずかつ)100で割り切れたら平年確定!
         return false;
     }
     // 以下は(400でも)100でも割り切れない場合
     if(year % 4 == 0) {
         // 100で割り切れないけど4で割り切れたらうるう年確定!
         return true;
     }
     // その他はすべて平年
     return false;
 }

敢えてコメント多めに付けてみました。
elseでつながず、1つずつ条件をチェックして確定した場合のみreturnで返しているので読みやすいですね。条件を一つずつ与えて篩(ふるい)にかけるイメージですね。こちらの方が最後のreturn false;に期待できますね(なんとなく)

条件が複雑になればなるほど、条件式はシンプルにわかりやすく書けないかと考えることが大事です。
読解しにくい条件式ほど厄介なものはありませんからね。

全くの余談ですが、例1~3はソースコードの見た目は違えどフローチャートにすればどれも同じになるんですよね(※return処理についての違いは省いています)。例3ではelseがなかったのですが、見えないelseは常にif節の後に付いているんです。

画像2

おわりに

プログラミングでバグが発生しやすい1つのポイントとして条件式が挙げられます。
特に複雑な条件式はチェックも難しくなります。もちろんテストケースを作ってすべてクリアすれば問題ないのですが、テストをするまでもなく判断できるコードであるに越したことはありませんよね。

条件式は意外と奥が深いです。

※サンプルはすべてテストをしていません。記事を書きながら、直接打ち込んでいます。参考にする場合は十分にチェックしてください。

この記事が気に入ったらサポートをしてみませんか?