if else 構文のインデントとコメントの書き方

プログラミング言語にほとんど必ず登場する「if・else 構文」のコーディングスタイルに関して話してみる。自分が想定する言語は Java メインな感じ。

1行 if の時にブレースを省略するか

if (buffer == null) {return;}

と

if (buffer == null) return;

は同じ動作をするということはお分かりかと思います。

正式な言い方があるのか分かりませんが、「省略が許可されている記述」というやつです。

わたしは「省略が許可されてても省略しない派」（←どういう派閥？）なので、常にカッコでくるむのですが、わたしのコードを読んだ人が「これってこういう書き方していいんだよ」とご丁寧にもカッコをはずしてしまったことがありました。

「それくらい知ってるんですけど」
「なら、なんでわざわざカッコつけてんの？」
「どっちでもいいんならカッコをつけた方が安全じゃないですか」

自分は必ずカッコ付ける派。というか、if 文の中身が1行でも必ず改行とブレースを書く派。

// 必ずこう書く。
if(buffer == null) {
  return;
}

中身が1行で終わる処理でも、いわゆる「ガード句」として return する場合でも、必ずこうする。

「省略形はバグを生む温床になるから使わない」と言ったんですが、相手は納得しませんでした。

「でも、省略形があるのに使わないってバカっぽくない？」
「何時間も原因を探した結果がカッコが足りないだけって方がバカっぽいですよ」
「でも、カッコがない方がカッコイイだろ」
「なんでカッコがない方がカッコイイんですか？」
「見た感じすっきりしててカッコイイ」
「でも、カッコつけてカッコつけないでバグだす方がカッコワルイですよね！」

確かに1行で処理が完結するのはスッキリしていてカッコイイ感じも分かるのだが、人間は全員バカなので、間違いが起きづらい方法を取るべきだと思っている。つまり、「ココは省略形で書いてるはずだから…」という思い込み、勘違いが原因でバグを出したり、それに気付かないでいたりするのは時間のムダで、ぼくからしたらその方がダサいのだ。

もう少しメリットを挙げるとしたら、省略形の1行で済ませていた部分を改修することになって処理を追加した場合の Diff が綺麗になる、ということだ。

// コレに「指定のレスポンスヘッダを設定する処理」を追加するとして、
if(buffer == null) return;

// こういう書き方にする必要があるが、Diff が多くなる。
if(buffer == null) {
  response.setHeader( ... );
  return;
}

// 最初から以下のように書いておけば、Diff は1行で済む。
if(buffer == null) {
  // ココに処理が増えるだけ
  return;
}

これは VB 系の言語でも同じ。

' こういうことしない
If buffer = Null Then Exit Sub

' 絶対こう書く
If buffer = Null Then
  Exit Sub
End If

1行省略形で書くより行がかさばるが、後で余計なバグ (= 自らの醜い過ち) を起こさないように、ぼくは自分のコーディングスタイルにも例外を作らない。if 文といったら必ずブレースを付けて改行を書くもの、としておくのだ。

コメントの書き方について

こちらの記事を見て、自分も思ったことを書いてみる。

こんな風に書く人が結構いてびっくりしました。

// ゼロの場合
if (value == 0) {
    funcA();
// そうでない場合
} else {
    funcB();
}

この書き方について筆者は、

ブロックもインデントも無視、修正したりコピーしたりするときにミスを誘発するし、こんな書き方をすることなんて絶対に有り得ないです。

と言っていて、自分は最初「そこまで言うことないだろ…」と思ったのだが、よくよくコードを見たらその意味が分かった。

つまり、上のコードで言うと、// そうでない場合 というコメントが説明するコード部分は else {} 部分であり、そのコメントが if の終わりのブレース } の手前に居るのはおかしいのである。

これがもし、こういう書き方だったら、まだ少しはマシだと思う。

// ゼロの場合
if(value == 0) {
  funcA();
}
// そうでない場合
else {
  funcB();
}

ぼくのコーディングスタイルではそもそも、} else if() { とか } catch() { みたいな、手前のブロックの終わりブレース } のすぐ後ろに次の構文を書かない。for 文の終わりとかの場合と比べると、なんとなく行間が詰まった感じがするから、プログラミングを始めた比較的初期の頃にこの書き方になった。

if(value == 0) {
  funcA();
}    // ← ココで改行する
else {
  funcB();
}

try {
  doInsert();
}    // ← ココで改行する
catch(SQLException e) {
  // Handle exception
}

だから、if else でコメントを書くなら、if の終わりのブレース } がある行と、else { の行の間に // そうでない場合 を書くことも、なくはないかな？と思っている。

ただ、自分が一番やるのは以下のスタイルだ。これなら if の場合と else の場合 (勿論間に else if が入った場合も) でコメントの位置が変わらないからだ。

if(value == 0) {
  // ゼロの場合
  funcA();
}
else {
  // そうでない場合
  funcB();
}

ただ、この場合、「if 文の条件に関してコメントをしているのか」、「if 条件に合致した時に何の処理をするか」のどちらについてコメントしているのかが若干分かりづらくなる。少なくとも自分にとってコメントは「これから登場するコードが何のために何をしているのか」を説明するモノ、と思っているからだ。だから、コメントの直前の行の説明をするとなると、若干違和感があったりする。

ぼくは元々コメントを多く残す方なので、ちゃんと書くならこんな風にするかも。

// □□のデータを △△する。
// value が 0 の場合、○○を××にしないと□□できないので、funcA() を実行する。
// そうでなければ通常どおり、funcB() を実行する。
if(value == 0) {
  // ゼロの場合
  funcA();
}
else {
  // そうでない場合
  funcB();
}

つまり、この if・else の塊が何をしようとしているのか、大まかな判断条件を先に書いておき、それから if 文を読んでもらおうとする感じだ。

どっちの書き方が優れているということではなく、要するにこういった細かい点に気を使いながらプログラミングすることって大事だな〜と思います。

なんでもガード句？else は不要？

「ガード句」という考え方がある。あるメソッドにおいて、異常系を先に処理して return しておくことで、if 文のネストを少なくするというやり方だ。

コードの中身はテキトーだが、if・else 構文の使い方を見てみてほしい。実際はもっと色んな条件分岐が重なっているものとしよう。

// こんな書き方をするとネストが深くなる
private String createMessage(String inputStr, String messageType) {
  String returnStr = null;
  if(inputStr != null) {
    if(messageType.equals("DEBUG")) {
      StringBuffer sb = new StringBuffer("<DEBUG>");
      sb.append(":");
      sb.append(inputStr);
      sb.append("...");  // 何かデバッグモード用の出力情報があるテイ
      returnStr = sb.toString();
    }
    else {
      StringBuffer sb = new StringBuffer("<INFO>");
      sb.append(":");
      sb.append(inputStr);
      returnStr = sb.toString();
    }
  }
  else {
    returnStr = "<NULL>";
  }
  return returnStr;
}

上のコードは、return 句をコードの最後にしか書かない作りにしてしまっており、なぜか変数 returnStr を使う必要がある。ネストも深くなりがちだ。どれが正常系 (一番よくあるパターン) なのか、特定の場合はどういうルートを通るのかが分かりづらい。

以下のように、異常系だとか、サクッと処理が終わるモノを先に return するようにすれば、else にあたるネストがなくなる。

// こんな書き方をするとネストが深くなる
private String createMessage(String inputStr, String messageType) {
  // 引数が Null ならサッサと終える
  if(inputStr == null) {
    return "<NULL>";
  }
  
  // デバッグモードなら別途処理
  if(messageType.equals("DEBUG")) {
    StringBuffer sb = new StringBuffer("<DEBUG>");
    sb.append(":");
    sb.append(inputStr);
    sb.append("...");  // 何かデバッグモード用の出力情報があるテイ
    return sb.toString();
  }
  
  // コレが正常系
  StringBuffer sb = new StringBuffer("<INFO>");
  sb.append(":");
  sb.append(inputStr);
  return sb.toString();
}

こうすると else にあたるネストがなくせたが、何でもかんでもこういう作りにするのがいいのかというと、実は else をあえて書いた方が意味が伝わりやすいコードになることがる。

elseを書いた方がよいケースもあるのではないか

もし、両方が正常処理ならば、ifとelseを持った条件記述を使うべきです。 if-then-else構造が使われるときは、if部にもelse部にも同じウェイトが置かれています。 これが、プログラムの読み手に対して両方とも等しく起こり得ること、等しく重要であることを伝えます。

if の場合が特殊で、else の場合が通常、といった、異なるパターンの場合分けであれば、ガード句を利用して else 句を書かないのが良いが、if の場合も else の場合も同じだけのウェイト (重要性) を持つ場合は、else をあえて書いてインデントを揃えて置いた方が伝わりやすい、ということだ。

無敵モードは特殊な状態だから、これはよいと思います。
しかし、マリオが大きいか小さいかの条件分岐についてはどうでしょうか？
マリオの大小はどちらが特別というものではなく、どちらも通常の姿だと思うので、else句を書いた方がわかりやすいと思います。

function() {
    if(マリオ.is無敵モード) {
        敵.死ぬ();
        return;
    }
    
    if(マリオ.isチビ) {
        マリオ.死ぬ();
        return;
    }
    else {
        マリオ.チビになる();
        return;
    }
}

たかが if 文、ではない。こういう些細なところから無頓着な人間が書くコードは総じて汚くて可読性が低く、不必要な処理も多く、ダッサいバグも多い。人のためにも、マシンのためにも配慮できておらず、正しい行いができていない。「そんなこと」と思った時点で、その人の質・レベルはそこまで。その程度の人間。ぼくはこうした話を少しもする気がない人間をプログラマともエンジニアとも思わないので、その時点で切り捨てている。

「それならこうした方がいいと思うよ」という持論があり、その根拠にそれなりの説得性が欲しいところである。「チームでこうだから」というのは理由にならない。チームの規約にするのであればチームのためになる規約にするべきだ。間違いを犯しやすいとか、効果の薄いルールなのであれば、ルールの方が間違っているのだから、ルールを変えるべきである。ルールだからとただただ従う人間はどんな理由があっても間違っている。

こうした「些細なこと」からも「何にとってどういう効果・メリットがあるのか」という視点を忘れずに、自分が何を選択すべきかを明確にしておきたい。

その他参考