最近はレビューしていることが多いなぁと思っている須藤です。レビューをしていると「真偽値を返す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」をちょいちょい見るなぁと思ったのでまとめてみました。

ツイートする

フォローする