長年プログラムを書いていても、良いコメントを書く技術ってぇのはなかなか身に付かないもので、
良かれと思って書いたコメントでも後々読み返すと書いた当人ですら首を傾げる始末。
長屋に住む八五郎もそんなSEの一人。
ある日、出社すると親方エンジニアがカンカンに怒っている。
どうやら先週末の退社前にレビューしてもらおうと push したコードについてらしい。
特に不味いところはなかったはずだが、はて、何が悪かったのだろうかと親方のデスクのモニターを覗き込むと──
<summary>
八五郎は普段から Visual Studio で C# のコードを書いている。
こいつにはXML Documentation Commentsというのがあり、
一定の書式に従って型やメンバーにコメントを書いておけば、
インテリセンスで有意義なメッセージが表示されたりAPI リファレンスの自動生成ができたりと、
大変便利な代物である。
さて、親方のモニターには以下のようなコードが表示されていた:
/// <summary>
/// ImportEntries
/// </summary>
private void ImportEntries()
{
...
}
「おい、八五郎、この <summary>
は何だ?」
「へぇ、親方。メソッド名でございます」
「何でこんなもん書いたんだ?」
「へぇ、親方。他のメソッドやプロパティに <summary>
があったもので、おいらも書かなきゃ不味いと思って書きました」
「で、これは役に立つのか?」
「へぇ、親方。インテリセンスを使う時に補足説明が表示されるので便利かと」
「ほほう」
「これが役に立つと?」
「へぇ、親方。こうして見るとさっぱり役に立ちそうにないです」
「なら何故書いた。こんなコメントには何も有意義な情報が含まれていないしコードを縦に間延びさせているだけでむしろ邪魔だ。せめてだな……」
/// <summary>
/// Foo用のデータをBarから取得し、Baz形式で保存する。
/// </summary>
private void ImportEntries()
{
...
}
「……こういう風に中身のあることを書いておけば……」
「……実際にこのメソッドを使うコードを書くときに役に立つってもんだ」
「へぇ、親方。でもそんな立派な <summary>
を書くのは大変ですぜ」
「おう。書く <summary>
が1個だけならともかく、全クラスの全メンバーの <summary>
を書こうとなると大変どころではない。第一、広く使われるライブラリーを書いているのでもない限り、細々としたメンバーにまで <summary>
を書いても見返りは少ない」
「へぇ、親方。するってぇとおいらはこれからどうすりゃいいんでしょう?」
「どうしても必要な場合はともかく、基本的にはクラスなりメンバーなりの名前だけでその役割が何か分かるようにして <summary>
は書かないようにしろ。むしろ<summary>
を書かないと分からないものは抽象化が適切行われていないと思え」
「へぇ、親方。もう少し具体的に言ってもらえると助かりやす」
「だからこの頭の悪い <summary>
をとっとと消せ」
「時に八五郎」
「へぇ、親方」
「こんなコードを見かけたらどう思う?」
i++; // iをインクリメントする
「へぇ、親方。プログラミングのプの字を齧り立ての中学生みたいなコードかと」
「ほほう。じゃあ先週末にお前が push したコードのこの部分だが、どう思う?」
private void ReplaceEntries()
{
// 一旦全削除する
DeleteEntries();
// データのインポート
ImportEntries();
}
「へぇ、親方。実に分かり易いコードだと思います」
「いや、プログラミングのプの字を齧り立ての中学生みたいなコードだろ。いいか……」
private void ReplaceEntries()
{
DeleteEntries();
ImportEntries();
}
「……こうすると分かり易いか? 分かり難いか?」
「へぇ、親方。こうして見るとどっちも大して変わりませんね」
「じゃああのコメントは必要だったのか?」
「へぇ、親方。こうして見ると全く必要ないです」
「そう。だからこういうコードを読めば分かることをコメントとして書かないようにしろってことだ」
「へぇ、親方。するってぇとおいらはこれからどうすりゃいいんでしょう?」
「今度からコメントにはコードで表現できないことを書くようにしろ。例えば実行環境の仮定だとか、今のアルゴリズムを選んだ経緯だとか、そういうことだ」
「へぇ、親方。もう少し具体的に言ってもらえると助かりやす」
「だからこの頭の悪いコメントをとっとと消せ」
#region
「で、八五郎よ」
「へぇ、親方」
「このコードなんだが」
protected void ButtonToAddEntry_Click(object sender, EventArgs e)
{
#region バリデーション
if (String.IsNullOrEmpty(name))
Fail("名前を入力してください");
if (String.IsNullOrEmpty(address))
Fail("住所を入力してください");
...
#endregion
AddEntry(new Entry(...));
RefreshEntryTable();
}
「この #region
は何だ?」
「へぇ、親方。こうすればコードを表示したときに……」
「……バリデーションのところだけ折り畳まれるので全体が見通し易くなります」
「ほほう。それは道理に適っているな」
「でしょう?」
「じゃあこのコードは何だ?」
protected void ButtonToAddMultipleEntries_Click(object sender, EventArgs e)
{
#region バリデーション
foreach (var p in params)
{
if (String.IsNullOrEmpty(p.name))
Fail("名前を入力してください");
if (String.IsNullOrEmpty(p.address))
Fail("住所を入力してください");
...
}
#endregion
foreach (var p in params)
AddEntry(new Entry(...));
RefreshEntryTable();
}
「へぇ、親方。こうすればコードを表示したときにバリデーションのところだけ折り畳まれるので全体が見通し易くなります」
「ほほう。じゃあこの2つの #region
の中身を見て気付くことはないか」
「へぇ、親方。どっちも中身は似たり寄ったりです」
「だったら #region
の中身の処理は……」
private void Validate(...)
{
if (String.IsNullOrEmpty(name))
Fail("名前を入力してください");
if (String.IsNullOrEmpty(address))
Fail("住所を入力してください");
...
}
「……というメソッドに括り出して、こいつを……」
protected void ButtonToAddEntry_Click(object sender, EventArgs e)
{
Validate(...);
AddEntry(new Entry(...));
RefreshEntryTable();
}
protected void ButtonToAddMultipleEntries_Click(object sender, EventArgs e)
{
foreach (var p in params)
Validate(...);
foreach (var p in params)
AddEntry(new Entry(...));
RefreshEntryTable();
}
「……こんな感じで使えば、 #region
で括る必要はなくなるし、重複した処理もなくなるじゃないか」
「へぇ、親方。するってぇとおいらはこれからどうすりゃいいんでしょう?」
「まずメソッド内に #region
を書こうと思った時点でその処理は抽象化が必要だと思え。メソッドとして括り出すなり、クラスとして括り出すなり。他にもこんなコードいっぱいあるぞ」
「へぇ、親方。もう少し具体的に言ってもらえると助かりやす」
「だからこの頭の悪い #region
をとっとと消せ」
親方のレビューが終わり自分の席へ戻った八五郎。
しかしその10分後に八五郎から再度レビューの依頼が。
「あの分量では到底10分で終わる仕事ではないはずなのだが」
と疑問に思いつつ親方が修正されたファイルを開くと──
「おい八五郎! このコードはなんだ!」
「へぇ、親方。『消せ』『消せ』と何度も言われたもんですから、とりあえず全部消した次第です」