2006-03-25

近況

転職後, ソースコードの品質に自信が持てない. ピアレビューをしていないからだ. 一人でコードを書いているとレビュー相手がいない. 自動テストとレビューは一度慣れてしまうとそれが無い状態がかなりストレスフル. デバッガが使えない環境で仕事をするのに似ている. 最近はようやく適応してきたが, "Radium Software" に ピアレビューの話が 載っているのを見て不安がよみがえった. いい機会から, 以前私のいたチームがどんな風に レビュー(ピアレビュー)をしていたかを少し書いてみよう.

なお, この方法自体は私や私のいたチームのオリジナルではなく, 社内のエンジニア達の間で広く草の根的に 発生したものだということを先に明らかにしておく.

レビューで手を抜く

私達のレビューのやり方は, オープンソースのレビュー様式を 企業内開発の特性に合わせて効率化したものだと考えることができる. たとえばあるオープンソースのプロジェクトでは, (一部の常連を除いて)まずメーリングリストなどに 変更の diff を投稿し, レビューを要請する. すると該当するレビュアが登場して diff を読み, 色々と注文をつけて差し戻す. これを何度か繰り返し, レビュアーが合格をだしたところでコードはコミットされる. 色々と短絡した亜種はあるけれど, これが基本的なワークフローだとおもう. (私は実際にオープンソースで開発をしているわけではないから 勘違いがあるかもしれない.)

この方法にはいくつか欠点がある.

その局所性から, 企業内でのソフトウェア開発は 分散開発であるオープンソースが持つこれらの問題を避けることができる. 私達のやっていた方法をいくつか以下に示した. これらの方法は例のごとく "正しい" やり方の 効率や生産性には遠く及ばないが, 始めるのは比較簡単. それに何もしない, あるいはアドホックなデスクレビューだけよりは効果がある. 不可能な完璧よりもより良い不完全を目指そう.

会議室で(だけ)レビューをする

まず会議室を確保しよう. それからレビュアを招集する. レビューをするための圧力をかけるためだ. メーリングリストに投げて「見ておいてねー」と言えば レビューをしてくれるなら苦労はない. 大半の同僚は自分の仕事が忙しく, 見向きもされないのが普通だろう.

レビューに会議室を使うのは, 通信教育ではなく予備校に通うというのに似ている. 場所が隔離され, 時間が確保されるのはとても大切だ. 観念してレビューをしようという気になる. レビューのたびに予約をするのは面倒で挫けがちだから, 毎日, 毎週というように定期的な予定として予約をいれよう.

形式的なレビューで要求される前準備はしなくていい. 大半の学生が授業の予習をしないのと同じで, 典型的なプログラマに前準備を求めるのは難しい. かわりにその場で各人がノート PC を開きコードを読む. この方法はお世辞にも効率が良いとは言えないものの, 皆が揃ってコードを読んでいると自分も読もうという気にはなる. (このへんの感覚も演習の授業と自宅での自習の比喩で理解できるとおもう.)

発見した不具合はみつけたはしから口頭で指摘しよう. 説明には適宜ホワイトボードを使うといい. 書記(じゃんけんに負けた人)は修正すべき点を結論を簡潔に議事録としてまとめる. 議事録に詳細な議論は必要なく, 問題の箇所を特定できる最低限の内容でいい. ホワイトボードを使った口頭での説明と議事録によって, 各人が解説のためのメモやメールを書くオーバーヘッドがなくなる. 議事録のコストは説明メールのコストよりずっと小さい.

こうして放置と対話コストの問題はだいぶ改善される.

なお, 後から指摘事項に対応した修正を追跡できるように 記録は BTS や Excel シートでまとめておくこと. 指摘したものをメールで送信するだけだと 修正しわすれても気付くことができず漏れがでる.

チェックインしてからレビューする

書いたコードはまずチェックインしてしまおう. 差分情報はレポジトリが保存しておいてくれるから, レビューでコードを読む時はレポジトリから差分を取得すればいい. 大半の SCM は差分閲覧の GUI を持っている.

コードに潜在的な問題を持ったままチェックインされるのは気分が悪いかもしれない. しかし企業内のプロジェクトはオープンソースのものと違って trunk HEAD の追っかけビルドが趣味の不特定多数などを相手にしなくていいから, 一時的に変なコードが入っても実害は少い. リリースまでに問題が解決していればいい. ただしチェックインからレビュー完了までの間が長いほど レビューで指摘された修正をするのが大変になる. (これは diff の陳腐化問題と似ている.) レビューは定期的かつすみやかに行おう.

レビューの運営が楽なのもこの方法のいいところ. SCM が時系列で差分を管理してくれているから, レビューはログを眺めて古いチェックインから現在に向って順次やっていけばいい. "レビュー指摘による修正のレビュー" というような, オープンソースでいう差し戻しの場合も通常と同じワークフローに組込めるため, 差し戻しを繰り返している間に diff が陳腐化する心配も減る.

溜まったバックログは諦める

納期が近付いてコードを書く速度が上がると, チェックインの速度がレビューをこなす速度を越えてしまい, レビューしているリビジョンと最新のリビジョンが離れていく. 休日出勤をした翌週などはかなりぶっちぎられる. リビジョンが離れるほどレビューからの修正は難しくなる. (陳腐化の問題.) また書いた本人も内容を忘れはじめる. こうしてレビュー実施の負荷があがり, 納期に追われてレビューに挫けるのはアンチパターンの一つだ.

教科書は, レビューを疎かにして納期やコードを優先するのは品質を重視しておらず 望ましくないと言うが, 現実的な解決策を示さない. それが事実だとしても納期や必須機能は変えられないのが現実だ.

こうなってしまったら, バックログ全てをレビューするのは諦めよう. かわりにいくつかの変更は妥協してスキップし, 最新のリビジョンに "追いつく" ことを優先する. もっとも素朴なスキップの仕方は一定期間のリビジョンをまるごと諦める方法. 最初はそれで仕方ないと思う. 何度か凝りたら次からは早めに手を打とう. どうでもよさそうな修正はスキップし, 大改造や際どい変更, 新機能の追加などバグの多そうなものだけをレビューしよう. 要するに検出するバグの数(やダメージ)の期待値を最大化するようレビュー対象を選ぶ.

"大物" だからとあとまわしにされ, なんとなくレビューされず風化する変更を何度か見たことがある. これではわざわざレビューの効率を下げることになりかねない.

...

このように 1. 会議室をリカレンスに予約し, 2. チェックインされた差分を眺めて, 3. 時々は妥協しながらレビューをすると, オープンソース方式のオーバーヘッドとビジネスからの圧力を乗り越えて なんとかレビューを続けることができる.

以下はおまけで, 単体テストがある時の補足. テストがあるとレビューはとても楽になる.

テストコードから眺める

レビュイはまずテストコードを示し, 今回の変更がどのようなものだったかを "外側" から説明しよう. レビュアにとって, 知らないコードの差分をいきなり眺めるのはしんどい. 書いたコードがどのように使われるかを知れば レビュアはその文脈に沿ってコードを読むことができる. 先入観なしに読むことで得られるレビューの批評性はいくらか損われるが, 変更の周辺を手がかりなしに読み解くのに比べるとずっと負担が少なく挫けにくい. またテストコードを精査することでテスト漏れをチェックできる利点もある.

困ったらテストに訊く

バグのありそうな怪しい場所をみつけたら, とりあえずレビュイに質問しよう. レビュイが即答できずに画面をじっと睨み始めたら, そのケースを検証するテストを書くことにして先に進もう. じっとコードを読みながら頭の中でプログラムを動かすのは 時間がかかるし間違えやすい. 人間が得意なのは他人の勘違いやポカミスを発見することで, 境界条件をチェックするのはどちらかというとテストの守備範囲. 脳の資源を酷使するのは挫折のもとになる. 節約しよう.

レビューの難しさ

手抜きテストと同様, 手抜きレビューもやらないよりはやった方がずっといい. とはいえ実際のところレビューで効果をあげるのはテストよりも難しい.

というのは, テストがもっぱら技術的な問題であるのに対して レビューは社会的な難しさを持っているからだ. レビュアを会議室に集めるのは自動テストを(こっそり)チェックインするより難しいし, ディベートに発展して先に進まないレビューは 失敗しない vector クラスのテストより性質がわるい.

レビューを始めようと思う人は "ピアレビュー 高品質ソフトウェア開発のために" の一読を勧める. これはあまり硬派なレビュー本ではないが, レビューにつきまとう社会的な難しさと それを乗り越えるためのガイドラインを示している. (だからなんとなく自己啓発本っぽい感触をおぼえる.) ガイドラインの方は(自己啓発本がそうであるように) やや現実味のない部分もある. それでも示されている問題は知っておくといい.

私がピアレビューを始めるまで : 履歴について 番外編

私はあまり社会的能力の高い人間ではないから, 当初レビューにはあまり積極的でなかった.

私が最初に参加したレビューは少しだけ手伝っていた巨大プロジェクトのもので, 出てくるコードはどれもさっぱり内容がわからなかった. 高校一年生が東大向け特訓コースの授業に紛れこんでしまったような感じ. 他のレビュアがコードを読んでいるあいだの沈黙がとても息苦しく退屈で, よく居眠りしていた. やがてだんだんサボるようになった.

そんな体験もあり, 私が二人のチームで新しいモジュールの開発を始めてからも しばらくはレビューをしていなかった. もう一人のメンバは新人だったから 彼の書いたコードに目を通そうとしていたものの, 実際には読んでいない部分もあったし指摘が面倒で手を抜くこともあった. 指摘をしてもそれを新人が私の期待したように反映するとは限らなかったが, 私のフォローアップはなかった. 面倒だった.

その後新たに参加してきた別のメンバーは私にレビューを勧めてきた. それでもしばらくレビューをすることはなかった. 締切が切迫しており, そんな時間はないと感じていたからだ. しかしその頃, 同じ部署にある隣のチームでピアレビューブームがおきていた. そのチームは私のいたチームより大所帯でレビューも賑やか. その様子がけっこう楽しそうなのを見て, 私もなんとなくレビューをしようという気になりはじめた.

そこで, まず何度かレビューの様子を見学させてもらった. 一回は参加者が 5,6 人のかなり賑やかなレビュー. 楽しそうではあったが, 今から思うと議論の時間が長過ぎて効率はあまり良くなかった気がする. ただ居眠りはしなくて済みそうだと感じたのを覚えている. 見学した別のレビューは実力者二人による高速レビューで, 背景説明などは一切なし. ものすごい速度でバックログが消化されていくのを目撃した. 同じくらいの能力の人間が文脈を共有した上でレビューをすると 高い効率が得られることがわかった. 人数は少い方が良いのも確かなようだった.

当時, 締切に追われた一連のやっつけ仕事によるコード品質の劣化を 私は危惧していた. チームメンバからの圧力も高まっており, 私達のチームもようやく重い腰をあげレビューを始めた. その時のチームは 四人. うち二人は隣のチームの影響もありレビューに積極的だった気がする. 私を除くもう一人は少し面倒そうな顔をしていたが, 特に反対もせず参加はしてくれた. (ただしばらくの間, 自分のチェックインが無い日のレビューには不参加だった.)

レビューを始めた当初は葛藤が多かった. 私達のチームが開発していたモジュールは私のコードが(量の上では)支配的で, 結果として私はそのモジュールを "私のもの" だと感じていた. だから他人のコードが自分のスタイルや期待していた設計と違うのは 気に食わなかったし, 自分のコードの問題を指摘されると不愉快だった. だから結構ゴニョゴニョとケチをつけたり言い訳をしたりしていた. それらは大半が嗜好の問題であり, そこで喧嘩をするのはピアレビューの代表的なアンチパターンだと言える.

ただ, いくつかの幸いに救われ挫折を逃れることができた. まず社内のコードは比較的コーディング規約がきっちり決まっていた. 従って規約に関するもめごとは少なかった. 規約の曖昧な部分でもめかけた時は, 社内のコードベースで多数派であるものや モジュール内の既存のコードに従うことにした. チームのメンバはみな人格者で, 私のわがままに寛容だった. 私がもう一人いたら破綻していたかもしれない.

そうこうしながらレビューを続けるうちに私の不満は薄れていった.

理由のひとつは, 大量に書かれた他人のコードを目の当たりにして, 自分の趣味を他人に強制するのが現実的でないと身に染みたため. 様々なスタイルが共存するのは当たり前なのだが, 自分のコードばかり眺めているとそのスタイルが一番だと 感じるようになってしまう. レビューはその感覚を変える.

もう一つの理由はメンバの書くコードが互いに似てきたためだろう. もともと他人のスタイルに合わせるのが得意なメンバもいたが, レビューをすると全体にその傾向が広まった. 普段から互いのソースコードを見ていて影響を受けるからかも知れないし, 他人の真似をする方が細かなスタイルを主張するより楽だと気付いたからかもしれない. 私もだんたんコードのスタイルや個性を主張する意欲が失せていった. これは今でも続いている. 既存のコードを真似する方がずっと楽なのだ. (.emacs の設定はいくらか面倒かもしれないが.)

やがてレビューで問題を指摘しあうのが完全に日常の一部になり, コードのどこかに問題があるのは当たり前という感覚が行き渡る. レビューはチームに根付き, 私にも根をおろした.

私達のチームは一般的なプラクティスに反して リファクタリングの提案などバグ以外の指摘もいくらか行っていた. これは喧嘩になっても不思議はないやりかただが, 実際にはさほど問題にならなかった. メンバの人柄も温厚だったし, 指摘されて直すのが当たり前になっていると "他人が変だと思うなら一応直しておこう" と考えるからだと思う.

とにかく, 私は運がよかった. レビュー文化のある会社, 部署だったし, チームのメンバは協力的だった. もしレビュー文化がなく殺伐としたチームにいたら, "ピアレビュー" を読んだとしても他の開発プロセス書籍の読後と 同じ失敗を冒したろう. つまり真似をして, 挫けて, それでおしまい. スタートアップの負担が少い手抜きのしかたを知っていて, かつ多少の失敗も協力によって乗り切れたから 挫折の壁を越えることができたのだと思う. 今の私がレビューをしていないのはその証左かもしれない.

だから私はレビューを素晴らしいと思う一方で, それをしない人々を非難する気にはなれないでいる.

洋書道

"Workin With..." 後半 100 ページくらいは前半の焼き直しなので飛ばしよみ. "ECMAScript ..." も同じく後半 100 ページくらい飛ばしよみ. こっちは built-in object のリファレンス. 年初の制約をちょっとだけ変更して仕様書は解禁. ただしページのあるものに限定.

13-19 の週は 1.5h 未達. なにか penalty を考えよう. 1h あたりいくらか寄付でもしようか ...