シンガポールでアジアのエンジニアと一緒にソフトウエア開発をして日々感じること、アジャイル開発、.NET、SaaS、 Cloud computing について書きます。

コメントが不要な分かりやすいコード

»

 私は人の書いたコードを読むとき、普通はコメントが書いてあっても見ない。中には、ひどいコードもあって、コメントを見ないと理解不能な場合があるが、それでもコメントは見ないで理解しようとする。コメントを信じたばかりに、ひどい目にあった経験が何度もあるからだ。

 コメントはプログラマが書くもので、文章能力の差が激しい。修正変更が繰り返されたようなコードは、たいていはコメントとコードが不一致になっている、また、やっている内容を理解する時、コメントを読むよりコードを読む方が早いことが多いということもしばしばある。

 私自身、コメントを書くことをプロジェクトのコーディングルールとしている場合は、しぶしぶコメントを書くが、その必要がない時は書かないことにしている。規模が大きければ大きいほど、参加するプログラマのばらつきが激しくなる。程度の低いプログラマの書いたコードをコメントなしで理解するのは至難の業だ。そこで、チームのルールとしてコメントを必須とするのは、理解できるし、そうあるべきだと思う。しかし、少数精鋭の優秀なプログラマばかりが集まったプロジェクトで、コメントを必須とするようなことはやめてほしいと思う。

 「コメントが必要ないコード」と、「ないと困るコードの違い」について考える。実際のコードで示せないこのような場では、分かりやすく説明することはかなり難しい。とにかく、下のような例で概念として説明してみた。ご理解いただければ幸いである。

 文字列のアレイ(moji_array)を、カンマをデリミタにした長い1つの文字列(いわゆるCSV)(longmoji)に変換する処理を考えてみる。そのプログラムを例1のように書くか、それとも例2のように書くかで、プログラマの傾向が分かる。例1の人は、プログラム全体を1つの大きな名前のない処理、つまりプロシージャと捉えている。それに対して、例2のコードは、少なくとも私にとって非常に分かりやすい。

 違いは、文字列をつなげる前に、コンマを付けるかいなかの判定で、処理を単純な流れとして記述する例1のコードは、単に意味のない数字、つまりIndex iが0か否かで判定する。それに対して、分かりやすいコードを書く人は、違いを意味のある英語の単語で表そうとする。それは例2の例では"started"である。

例1(ナイーブなコード)

StringBuilder longmoji = new StringBuilder();
for( int i =0; i < moji_array.Length-1; i++ )  
{
  if (i == 0) 
  {
    Longmoji.Append(moji_array[i]);
  }
  else 
  {
    Longmoji.AppendFormat(",{0}",moji_array[i]); 
  }
}

例2(意図が分かりやすいコード)

StringBuilder longmoji = new StringBuilder();
bool started = true; 
foreach( string moji in moji_array )  
{
  if( started )   
  {
    longmoji.Append( moji_array[i]);
  }
  else 
  {
    longmoji.AppendFormat(",{0}",moji_array[i]); 
  }
  started = false; 
}

 要するに、分かりやすいコードを書く人は、とにかく変数、何かの判断で分岐するときの論理、ある処理の塊、などに明確な名前を付けようとする。さらに、上の例にはないが、処理をできる限り小さい処理の塊に分離しようとする。頭の良い人は、一度に頭に入れて処理できる情報量が多いのだろう。そのため、かなりの大きさの処理を頭の中で一気に構築して、それをプログラムとして記述することに何の苦も感じない。結果、処理を分離し、さらにそれに明確な名前を与えるような方法の必要性に気付かない。また、経験が少ないと、それほど複雑になる処理のプログラムを作った経験が少ないこともあり、必要性に気付かないということもあるだろう。経験の長い人は、その経験の中で痛い目にあった経験があり、その結果、大きくて複雑な処理をできる限り分割するように注意するようになる。

例3(宣言的要素をとり入れた)

StringBuilder longmoji = new StringBuilder();
foreach( string moji in  moji_array ) 
{
  longmoji += (longmoji.Length == 0 ? 
                  moji : "," + moji);
}

例4(LINQを活用)

StringBuilder longmoji = new StringBuilder();
moji_array.ToList
().ForEach(moji => 
 longmoji.AppendFormat( longmoji.Length == 0 ? 
                           "{0}" : ",{0}", 
             moji ));

 ついでと言ってはなんだが、例3ではコードに「宣言的」要素をとり入れてみた。最初の行で、カンマを入れるか入れないかの判断を、例2では処理の流れ、つまりif文を使った処理の分岐で表すが、例3では「宣言」で表している。同じことだと言う人もいるかもしれないが、例2と例3のコードは、考え方がかなり違う。

 例4は、その流れをLINQを使って、さらに徹底したものだ。同じことをやるために、最終的には2行になっている。しかも、そのうちの一行は変数の宣言なわけで、実質的には1行だ。

 そして最近の私のコードはこれをさらに徹底して、以下のようにすることが多い。最新のC#で導入された『拡張クラス』を使っている。ここまで来ると、確かにコメントは不要だと分かってもらえると思う。

例5(拡張メソッドで完ぺき!)

別のアセンブリ(dll)に次のようなUtilityを準備。そこに文字列のアレイの機能を拡張するメソッド“ToCSV()” を実装しておく。

public class Utility
{
  public string ToCSV(this string[] sts)
  {
    StringBuilder longmoji = new StringBuilder();
    sts.ToList
().ForEach(moji =>
       longmoji.AppendFormat( longmoji.Length == 0 ? 
                                 "{0}" : ",{0}",
                              moji );
       return longmoji.ToString();
  }
}

処理部では、Utilityで実装されているToCSV()を使うだけ。

  longmoji =  moji_array.ToCSV();   
Comment(9)

コメント

var longmoji = new StringBuilder(string.Join(",", moji_array));

ForEachを使った実装では、たとえばnew[] { "", "", "hoge" }のような入力を与えた際に"hoge"となってしまいますが、これは想定通りなのでしょうか?
また、ToListを行ってForEachを呼び出すくらいなら、普通にforeach文を使った方がわかりやすいと思います。
この実装でいいなら、自分だったらAggregateを使って実装します。

var longmoji = Aggregate(new StringBuilder(), (acc, s) =>
acc.Length == 0 ? acc.Append(s)
: acc.AppendFormat(",{0}", s));

こちらの方がより「宣言的」と思いますがどうでしょう?

もちろん、anis774さんのstring.Joinを使った方法が最もわかりやすく、最善の方法だと思いますが。

nanashi

自分はこうだった、すみません…

StringBuilder longmoji = new StringBuilder();
if (moji_array.Length > 0) {
longmoji += moji_array[0];
for (int i=1;i

haiiro_shimeji

例がいまいちなのでは。。

3. 4. の longmoji.Length == 0 は「CSV文字列に要素が未登録であるかどうか」を示しているのだと思いますが、これについて「変数、処理の名前による説明」が無く不親切に感じますし、本質的には「CSV文字列に要素が未登録である≠バッファの長さが0」であるため、バグの温床にもなります。

自分としては、お題に対する解決としては 2. がベストだと思いました。
「CSV文字列に要素が未登録であるかどうか」を正しく判断できる処理を作り、それを説明できる名前がついていれば 3. 4. のような書き方が活きると思います。

K.Oumi

こんにちは。

拡張メソッドいいですよね~。
自分はVBの属性を付与する記述のほうが好みです。

yama

このコードにコメントなかったら泣くわ。
joinが意図ならjoinで書こうよ。

sasaki

絶望した。

コメントがたくさんついて、結構プロラマが読んでいると分かり感動しました。
順番にレプライします。

anis774様。  string.joinなんてものがあるとは知りませんでした。今度から、
使えそうです。どうもありがとうございました。

bleis-tift様。『実に興味深い!』遅ればせながら、最近ドラマの『ガリレオ』をネットで見ました。 主人公の湯川先生先生の口癖です。確かにAggregateを使ったほうがずっと宣言ポイです。こんどからそうします。私のコードの欠点を見事に指摘ありがとうございます。bleis-tiftさんのような人と一緒に仕事をしてみたい。

nanashi 様
私も、私よりもっとすごいプログラマに怒られてます。頑張りましょう。

haiiro_shimeji様
はい。私もその点に懸念がありました。しかし、無理やり宣言的書き方に
もって行こうとして、逆にコードの明示性を犠牲にしてしまっています。確かに、2の方が明示性が高いです。

K.Oumi様
はい。拡張メソッドは良いです。欠点は、何をincludeをしたらよいのか
わからなくなってしまう可能性があることです。クラスライブラリがしっかりと体型だっていないと、いけません。
VBの属性云々を、もしよかったら教えてください。VBはあまりなれないもので、すいません。

yama様
はい。こういうコードに得てしてコメントが無いのです。それが私の今までのプロジェクトの現実です。Joinが意図ですが、Joinの存在を知らなかった私の意図はToCSVだったということで許してください。今回はJoinと言う便利なものがたまたまあり、一発で実装出来てしまう、より分かりやすく、より短いコーディング方法があったため、話が知っているか、知らないかの方向に言ってしまいました。例の選択が悪かったようです。すいません。

sasaki様
絶望しないでください。すいません。

がると申します。
「コメントがなくても理解できるくらいに美しいソースコードを書く」事はとても大切だと思いますが、それを踏まえてなお、業務であれば「何をしたいのか、という意図を、丁寧にコメントで書いておく」事は必須なのではないか、と思いますが如何でしょうか?
http://d.hatena.ne.jp/gallu/20101116/p1

コメントを投稿する