最近はレビューしていることが多いなぁと思っている須藤です。レビューをしていると「真偽値を返すif/else」をちょいちょい見かけるなぁと思い出したので18回目のノータブルコードとして紹介します。17回目のノータブルコードが2021年6月28日なので、3年強ぶりのノータブルコードです。久しぶり！
「真偽値を返すif/else」というのはこういうif/elseのことです。私は知らないのですが、なにか名前がついていたりします？
static bool
is_odd(int x)
{
if (x % 2 == 1) {
return true;
} else {
return false;
}
}
こういうif/elseを見たときはif/elseなしでこれでいいんじゃない？というコメントをしています。
static bool
is_odd(int x)
{
return x % 2 == 1;
}
それではこんなif/elseについて考えてみましょう。
if/elseなしのほうがよいのか
私はない方が読みやすいなぁと思います。
static bool
is_odd(int x)
{
if (x % 2 == 1) {
return true;
} else {
return false;
}
}
というようにif/elseがあると、x % 2 == 1がtrueならtrueを返して、そうじゃないならfalseを返すのね、と読んでいる気がします。
一方、if/elseがないとどうでしょう。
static bool
is_odd(int x)
{
return x % 2 == 1;
}
この場合は、x % 2 == 1かどうかを返すのね、と読んでいる気がします。
考えることが1つ減っているので読みやすく感じている気がします。
このケースではtrueのときにtrueを返していますが、次のようにtrueのときにfalseを返している場合はどうでしょうか。
static bool
is_odd(int x)
{
if (x % 2 == 0) {
return false;
} else {
return true;
}
}
読むときにもうひと手間かかっていそうな感じがしますね。
ということで、私はif/elseがない方が読みやすいと感じます。
いつもif/elseがない方がよいのか
それでは、いつもif/elseを使わない方が読みやすいのでしょうか。私の経験ではそういうわけでもなさそうです。
たとえば次のケースはどうでしょうか。if/elseではないですがifは使っています。
bool
grn_obj_is_bulk(grn_ctx *ctx, grn_obj *obj)
{
if (!obj) {
return false;
}
return obj->header.type == GRN_BULK;
}
これは次のようにも書けます。
bool
grn_obj_is_bulk(grn_ctx *ctx, grn_obj *obj)
{
return obj && obj->header.type == GRN_BULK;
}
このくらいのコードならどちらでもいい気はしますが、私がコードを読む人なら、コードを書いた人の意図を違うように読みそうです。
もとのコードではifを使ってNULLのケースを除外しています。（early returnでfalseを返しています。）そのため、NULLのケースはこの関数では本質ではないのだろうと読みます。returnのところにあるobj->header.type == GRN_BULKの部分が本質なんだろうと読みます。
returnだけにしたコードではそのまま「objがNULLじゃなくてobj->header.type == GRN_BULKなときだけtrueなんだな」と読みます。それぞれの条件の重要度の違いはあまり感じません。
ちょっとすぐに具体例を見つけられなかったのですが、&&と||が入り乱れるような複雑な条件の場合やはifを使ってearly returnした方が読みやすいことが多いです。
まとめ
「真偽値を返すif/else」をちょいちょい見るなぁと思ったのでまとめてみました。
