先日おいらが中心になって開催したコードレビューコンテストとLIVEリファクタリング(本来の目的は解答例説明会)の内容を報告書風にまとめてみます。
結構好評だったので、メンバーのスキルアップやモチベーション向上に悩んでいる方は参考になるかもしれません(^ ^)
要約
きっかけ
数ヶ月前に以下のような簡単なWebアプリを作成した。
- 要求: Excelで管理しているタイムシートから開発案件ごとの工数を集計して出力する
- 仕様: Excelの内容をコピペで貼付け→テキストを解析→集計結果をテキストで表示
- プラットホーム: ASP.NET 2.0 (C#)
ただし、最初は自分の仕事効率化のために作ったユーティリティツールだったので、コードはかなりやっつけで作った。
自分以外のメンバーも使えそうなことが分かったので、グループミーティングでそのツールを紹介した。
つまり、アプリケーションの仕様はここで全員に理解してもらった。
自分では自分のコードのどこがダメで、どうリファクタリングすべきかは大体分かっていた。
しかし他のメンバーにはC#のコーディングスキルやリファクタリングに関する知識がそれほど高くない人もいた。
そこでこのダメコードを題材にして、他のメンバーにスキルアップしてもらうことを思いついた。
コードレビューコンテストの内容
以下のような条件でコードレビューコンテストを告知し、応募を募った。
- コードを公開する(全部で180行程度)
- 何でも良いのでダメな点や改善可能な点を指摘し、提出してもらう(コードレビュー)
- 提出してもらったレビュー内容を出題者が採点する
- 指摘事項1つにつき、ポイントは1点とする
- ただし鋭い指摘事項があれば任意のボーナスポイントを与える
- 反対にあまり意味がない指摘事項であればポイントはゼロとする
- 獲得したポイントが多い順にランキングを付ける
- 1位になった人には景品をプレゼントする(モチベーション向上のため)
- プレゼントはケチケチせず、自腹で買う(最終的には上司が援助してくれた)
- 期間は土日を挟んで4日程度(長くしすぎてもダラダラしそうなので)
- 参加は任意とする(強制ではない)
- 気軽に参加してもらうためあまり堅苦しいものにせず、指摘事項はどんな小さいものでもかまわないとする
さらに解答例を説明するのに改善前と改善後のコードを見せるだけでなく、コードが改善されていく過程やプログラマーの思考も参考にしてもらうために、他のメンバーの前でリアルタイムにリファクタリングしていくことも思いついた。
解答例説明会(別名LIVEリファクタリング)の内容
以下のような条件で解答例説明会を告知し、メンバーの参加を募った。
- 時間は業務終了後の2時間程度
- 会議室を借りて、プロジェクターにコードを映しながらリファクタリングを行う
- 解答例は事前に準備せず、その場で考える
- うまくいくところだけでなく、間違えたり試行錯誤したりする様子を見てもらうのも勉強になると考えた
- 参加は任意とする(強制ではない)
- レビューを提出していないメンバーも参加可能とする
- 業務やプライベートの都合に合わせて途中の出入りは自由とする
- リラックスしてもらうためにお菓子と飲み物を用意する(代金は一人200円のカンパで補う)
- 提出されたレビューの採点や順位付けは事前に行っておく
- 最後の30分ぐらいを使ってメンバーから提出されたレビュー内容と獲得ポイントを公開する
- 説明会参加者の異論がなければ順位を確定し、1位のメンバーに景品を渡す
コードレビューコンテストの実施結果
10人中5人からの応募があった。
採点結果は下が3点、上が7点だった。
出題者としては10箇所以上の指摘事項が上がってもおかしくないと思っていたが、思惑通りにはいかないようだ。
メンバーから挙がった主な指摘事項は以下の通り。
- マジックナンバーを使わず、定数に移動させる
- 外部の都合で変更されやすい値はハードコードせず、外部の設定ファイルに移動させる
- 変数名をわかりやすいものにする
- 長過ぎるメソッドは分割する
- 深いネストを避ける
- 多すぎるswitch文による分岐をなくす
- コメントが全くないので適宜コメントを入れる
解答例説明会の実施結果
こちらは10人中8人の参加があった。
約2時間の間に徹底的にリファクタリングを行い、さらにシステムの機能追加までしようと考えていたが、実際は思ったようにリファクタリングできない部分もあり、当初改善しようと思っていた箇所をすべて改善することができなかった。
ただし上で述べたように、うまくいくところばかりではなく、開発者が試行錯誤している様子を見てもらうのも勉強になると考えていたので、これはこれで問題はない。
また、説明会ではみんなで問題点を解決するためのアイデアを出し合ったり、新しいロジックの長所や短所を議論したりできたので、非常に密度の濃い時間となった。
終わってみると説明会というより、ペアプログラミングならぬ「グループプログラミング」を実施していたような気分になった。
説明会の流れはだいたい以下の通り。
- 大まかなリファクタリングの方向性を説明
- SourceMonitor*1で現行のソースの複雑さやネストの深さなどを測定
- 複雑さを現行の最大24から10以下へ、ネストの深さを最大8から4以下へ下げることを目標とした
- Smart UIアンチパターン*2になっていたので、ロジック部分をクラスライブラリに移動
- デグレードを防止するためにNUnitを使ってロジック部分に対する簡単なユニットテストを作成
- リファクタリングを開始
- リファクタリングしても「不吉な臭い」が完全に消えないロジックがいくつか出てくる
- だんだん説明会というよりも、最善のロジックを追求する議論の場になってくる
- なんとか複雑さとネストの深さを目標値まで下げることができたが、マジックナンバーが残っていたり、リファクタリング後のロジックもどこかしっくりこないものがあったりした
- リファクタリングしきれなかった部分については、各人で改善してもらっても良いことにした
- 最後に提出された5件のレビューを全員で確認し、一位のメンバーに景品を渡して終了した
メンバーの感想
コンテストや説明会について、以下のような感想がメンバーから寄せられた(すべて原文の通り)。
僕にとって、とても刺激的な活動でした。
参加してみた結果、C#で書いた事が無いので、やっている事を理解するので精一杯でしたが、
あの日は帰宅後、腰の重い僕でも「何か始めなければヤバイ」と思い、とりあえずすぐにMACでも触れるobjectiveCのiOS開発環境を再作成したり参考書を見たりしました。
確実に参加者に良い影響を与えたのでは無いかと思います。
それぞれのknow-howを共有したりする事で、刺激しあえると思うので、こういった活動は継続して行ったほうが良いと思いました。
とても良かったです！このような活動は続けたいと思います。
TDDを本から勉強するとゼロから各例が多いですので、既にあるものにテストをどう追加するかがあってとても実用的でした。
これからTDD、Refactoringそして、既存のものにテストを追加する話題はよく出てくると思いますので、またやりたいです！
今日は時間ありませんでしたが、ソースを自分で見て、Refactoringできるか見たいです。
いろいろと、新しいC#の使い方のテクニックにも触れた感じがします。
また、NUnitについてもよく理解できました。
テストの自動化も、手続きが面倒そうで、今まで前向きに考えていなかったのですが、
NUnit、その点よく考えれてており、使えそうですね。
なによりも、後の開発の効率につながるため、使うべきツールだと感じました。
積極的に活用していこうと思います。
ありがとうございました。
とてもいい企画＆結果だったと思います。ありがとうございました。
開発者が心がけていること、思考過程を実際に感じるのはいい勉強であり、刺激です。
やはりあのぐらいの時間（2，3時間）を掛けてやっと一区切りだと思うので、
時間は取られますが、参加者全員にとって元は取れたと思います。
楽しくまた価値ある時間を過ごす事ができました。
個人的な率直な感想として、、、
身の丈を知るいい機会になりました。 自分はまだまだだと・・・
自分はどちらかというと、受身に回ってた感がありましたが、
詰まった時にこれ以上どうやったらリファクタリングするのか？？と思いながら参加していました。
また個人的にはNUnitの使い方が気になっていたので、その雰囲気を知る事ができてよかったです。
自分もまだ勉強中ですがどこかで見積もりについての勉強会を開催できればなぁ・・・なんて思った今日この頃でした。
だた、これはどちらかというと勉強会というより、プロジェクトを組んでするのがいいのかな？？どうなのかな。。。っと思ってます。
開催していただき、ありがとうございました＾＾
とても良かった、の一言に尽きます。趣旨にもあったツールの使用方法、実施者の思考過程の理解も、十分満足するレベルで見ることが出来ました。
行き詰ったことも「あぁ・・・こうすればリファクタリングは無理になるのか」といういい意味でみんなの糧になったのではないかと思います。
今回は伊藤さんの得意なC#での説明会でしたが、似たようにPerl*3やDatabase関連もあれば、メンバーの更なる飛躍に繋がるのではないかと思います。
