ITを活用して事業の課題を解決するサービス「納品のない受託開発」を提供する会社、ソニックガーデンの西見公宏（にしみ・まさひろ／@mah_lab）です。お客様の「バーチャルCTO」として、サービスの企画からシステムの開発・運用まで、日夜幅広く関わらせていただいております。
皆さんは普段、ソースコードをどのくらい読んでいるでしょうか？
普段からソフトウェア開発をしている人であれば、何か問題が起こったときの原因調査のために他の人が書いたコードを読んだり、はたまた自分の書いたコードを読んだりする機会は多いですよね？ オープンソースのライブラリも当たり前のように使っているでしょうから、実装の詳細を確認するためにライブラリのソースコードを読む機会も多いと思います。
このように、ソフトウェアの開発をする上では、ソースコードを書く技術もさることながらソースコードを読む技術も当然求められるものです。
では、ソースコードを読む技術はどのようにして鍛えられるのでしょうか？ オープンソースのライブラリのソースコードを片っ端から読むのもひとつの手ではありますが、個人的な意見では、コードレビューこそ、ソースコードを読む技術を鍛える絶好の機会だと考えています。「レビュー」というと、通常はベテランが品質の維持・向上のために行う活動を指すため、若手にレビューの機会が回ってくることは少ないかもしれません。
しかし、コードレビューによってどのような問題が解決されるか？を考えると、「コードレビューはベテランがチェックするのが一番効率が良い」というアプローチが間違っているのではないか？ と私は考えています。
本稿ではコードレビューによって解決される問題の例を確認した後、実際にチームでコードレビューを実施する上で気をつけるべきことを紹介します。コードレビューをしていない現場から、ベテランが一方的にコードレビューを実施している現場、または、すでに実施しているがイマイチ効果が出ていない現場まで役に立つ情報だと思いますので、ぜひご参考にしてみてください。
そもそも、何のためにコードレビューを行うのか？
そもそもコードレビューとはどんな問題を解決するために行う活動なのでしょうか？
普通に考えれば、不具合を発生させるソースコード──例えばある特定条件下でエラーになってしまうコードや、著しくパフォーマンスを劣化させ、障害につながりうるコードなど──を、人間の目で確認し除去する活動のことを指すと思います。
コードレビューで検知しうる全ての不具合がテストコードによって担保されていれば、コードレビューを行わなくてもいいのでしょうか？ 私はそうは思いません。
なぜならコードレビューの最大の目的は、ソフトウェアを継続的に開発するために、開発そのものの継続性を確保することだと考えているからです。
ソフトウェアは継続的に開発することが前提
エンジニアHubをご覧の皆さんの多くはWebエンジニア、もしくはスマートフォン向けアプリエンジニアの方々だと思います。そのため、Webアプリケーションやスマホアプリを開発しているという前提で話を進めますね。
Webアプリケーションやスマホアプリは大概の場合、開発してローンチしたあとに全くソースコードをいじらない、なんてことはないと思います。新機能追加や不具合の改修のほか、Webアプリケーションであれば利用しているフレームワークや処理系のバージョンアップ、スマホアプリであれば配信先のOSのバージョンの変化に追随する必要があるでしょう。
ソフトウェアそのものの改修といった内側の変化もあれば、デプロイ先のOSのバージョンアップなどに伴うソースコードの変更といった外側の変化もあるわけです。
従って、内側の変化がなければ全くソースコードを変更しなくていいのか？ と言われるとそんなことはなく、誰かにアプリケーションを提供しているならば、その誰かがそのソフトウェアを使い続ける限り、ずっとソースコードの変化は続くのです。つまりソフトウェアは継続的に開発し続けるものという前提があるのです。
継続的に開発するためには「手の入れやすさ」が重要
開発が継続されるものだと考えると、いかにソースコードに手を入れやすくするか？というのは品質上のひとつの課題になります。
なぜなら手の入れにくいソースコードである場合、開発効率はもちろん落ちますし、手を入れた結果、気づかずに埋め込まれる不具合も恐らく厄介なものでしょう。関連ソフトウェアの脆弱性対応もままならず、セキュリティリスクを晒し続ける可能性すらあります。そうした悪いコードを塩漬けにした結果、全く手を入れることができなくなり、最終的にはリプレースするしかない……データ移行なども考えると、非常にコストのかかる決断になります。
ソースコードに手を入れるのはもちろん人間です。だからといって「万人が手を入れやすいソースコードを目指す」というのは難し過ぎる問題ですね。せめてソフトウェアをメンテナンスしていく人間、もしくはメンテナンスしていくであろう人間にとって、手の入れやすいソースコードであればいいでしょう。
「手の入れやすさ」はどのようにして評価できるものでしょうか？ 処理の複雑さの指標であるサイクロマティック係数*1を使うのもひとつの答えかもしれませんが、最終的にそのソースコードに手を入れる人間、例えば同じ開発チームのメンバーたちに、実際にソースコードを読んでもらうべきでしょう。
とはいえ、ただ読んでもらうだけでは改善につながりませんよね。読んでみて「もっとこうした方が良い」と思った部分や、純粋にコードを読んだだけでは意図が分からなかったところなどを情報交換し、時にはあるべき姿をディスカッションし、みんなが手を入れやすい形に改善していくべきでしょう。
もうお気づきかもしれませんが、筆者はこの改善活動を「コードレビュー」だと考えています。「ベテランだけがコードをレビューして、若手がコードのレビューに参加しないのは間違いだ」と述べた理由はここにあります。
ソースコードに手を入れるのは、そのソースコードに関わる全てのエンジニアなのですから、ベテラン・若手に関わらず、みんなが読めるソースコードであるべきですよね。
ここまでのまとめ
この章ではコードレビューを行うそもそもの目的について考えました。この章で述べたことをまとめると以下のようになります。
- ソフトウェアは継続的に開発し続けるのが前提
- その前提を受けて、コードレビューの最大の目的は「開発の継続性を確保すること」
- 開発チームにとって手の入れやすいコードでなければ開発の継続性は確保できない
- そのためコードレビューにはベテランも若手も分け隔てなく参加するべき
では具体的に、どのようにして「コードレビュー」という活動を行うのがいいのでしょうか？ 筆者の勤める株式会社ソニックガーデン（以下、ソニックガーデン）での実践例を挙げながら、解説していきたいと思います。
ソニックガーデンはソフトウェア開発の会社で、主に「納品のない受託開発」という受託開発のサービスや、リモートワークの課題を解決するチャットツール「Remotty」といった自社製品の開発・販売を行っています。Remottyを利用しながら社員全員がリモート（在宅）で働いているため、コードレビューにおけるコミュニケーションも基本的にインターネット上で完結しています。そのため「対面で」と言う場合はオンライン会議ツール越しのことを暗に示しておりますので、その点だけご注意ください。
コードレビューを始める準備〜ルールを決める
いよいよ「さあ、コードレビューを始めよう！」といったところですが、チームでコードレビューを運用していくに当たってはいくつか決め事をしておくとスムーズです。
コードレビューの目的、観点を共有する
まずは コードレビューの目的、観点を共有する必要があります。「共有する」ことは非常に重要なポイントです。共有なき状態でコードレビューを回そうとすると、レビューする側とされる側の間ですれ違いが起こってしまいます。
「変数名にいちいちケチをつけられて気分が悪い」 「可読性が下がるような細かいパフォーマンスチューニングの話について指摘されても困るんですけど」 「ライブラリ変えた方がいい、って今話すタイミングなの？」
……など、両者の間ですれ違いが起こる原因はいくつもあります。何を期待してコードレビューが依頼されているのか、レビューする側もしっかり認識しておくべきですよね。コードレビューで解決すると決めた問題、コードレビューでは扱わないとした問題の解決方法をキッチリ決めておくと、レビューはスムーズに回ります。
みんなの目を通してメンテナンスしやすいコードにしていく、という前提のもと、自分たちにとってメンテナンスしやすいコードとは何か？ と考える必要があります。これは一般論で考えず、チームで話し合って決めるのがいいでしょう。また、一度話し合って決めたものを守り続けるというよりは、レビューをする中で「これはこっちの方針の方が良いのではないか？」といった議論が起こる度に見直していくと、より洗練されていきます。
コードレビューをどのタイミングで行うのかを決める
コードレビューの目的に応じて、どのタイミングでコードレビューを行うのかを決めるといいでしょう。Aという目的に向かっていくために、どのタイミングでチェックポイントを設けるべきか？ という考え方です。
具体的な例を挙げると、私たちは「リリース」を中心に物事を考えています。安定してリリースを続けていくためには、リリース時に事故が起こらないことはもちろん、万が一起こってしまった場合でも迅速にリカバリできるようになっている必要があります。
迅速なリカバリのためには、リリースされた差分が何を変更したものなのかをコードから読み取れなければなりません。そのためコードの可読性はしっかり確保すべきですし、リカバリ自体が困難にならないよう、実装レベルの設計に不具合がないかもレビューします。
つまり「安定したリリースを行うためにメンテナンスしやすいコードになっているか」をチェックしているので、「リリース前」というタイミングでレビューを行っているのです。このように、目的によって適切なタイミングも変わるので、単純によそで上手くいっているタイミングを採用するのではなく、目的を見据えて自分たちでタイミングを決めましょう。
コードレビューを行う単位を決める
1回当たりのレビュー量の目安も決めましょう。
単純に「レビューはリリースの前だけ」としてしまうと、場合によっては差分が大き過ぎてレビューが困難になります。経験上、レビュー自体の品質も考慮して、コードレビューの単位はできるかぎり小口化するべきだと考えています。せめて1回当たり30分程度で読める量でありたいところです。
私たちもレビュー単位の小口化と、定期的なリリースによるリリース時の差分そのものを減らす運用を標準とし、その運用がどうしても難しい場合には差分をフィーチャー単位に分割してレビューを行います。このようにレビューを進めていくに当たって、どのようなブランチマネジメントを行っているかについても紹介しましょう。
最大で、以下4種のブランチを切ってマネジメントを行っています。
-
featureブランチ- 機能別に区切られたブランチ
-
stagingブランチ- テスト環境にデプロイして動作確認するためのブランチ
-
release-candidateブランチ- 本番リリース候補のフィーチャーをマージしたブランチ
-
masterブランチ- 本番環境にデプロイするブランチ
そして以下のルールでブランチを運用しています。
- masterブランチのものはいつでも本番にデプロイ可能である
- stagingブランチのものはいつでもテスト環境にデプロイ可能である
- 新しい機能を開発する際は、説明的な名前のブランチをmasterから作成する（remove-teamなど）
- 作成したブランチにローカルでコミットし、サーバー上の同じ名前のブランチに定期的に作業内容をpushする
- featureブランチの内容をサーバー上で動作確認したい場合はstagingブランチにマージする
- 何らかのアドバイスが欲しいとき、テスト環境での動作確認が完了した際にrelease-candidateブランチに向けてプルリクエストを作成する
- メンバーがレビューをしてOKを出してくれたら、差分をrelease-candidateにマージする
- リリースのめどが立ったらrelease-candidateブランチをmasterブランチにマージする
- masterにマージされたら直ちに本番にデプロイする
1〜2名しか関わらないプロジェクトではプロセスを簡略化してstagingブランチに差分を直接コミットし、stagingからmasterブランチにプルリクエストを作成してリリース前レビューを行っています（stagingブランチ＝release-candidateブランチという運用）。
シンプルな運用だと思いますが、いずれの方法にしろ、ポイントはコードレビューの小口化です。できるだけ細かい単位でコードレビューできるように運用することでコードレビューが小口化され、1回に見る差分が少なくなることでレビューの品質が上がり、コードの改善も円滑に回せると考えています。
コードレビューの担当の持ち回り方を決める
最後に、コードレビューを誰がするか？ を決めましょう。チームのみんながメンテナンスできるコードにするために、可能な限りレビューを持ち回り制にするのをおすすめします。
私たちの例では、多数のメンバーが関わっているプロジェクトというのはそう多くありません。1〜2名でコードを書いているプロジェクトが大半で、そういったプロジェクトを3人程度のチームという単位で複数管理しています。
そのため「リリースするためには誰かにレビューしてもらう必要があるが、他の人のプロジェクトをレビューしていない状態でレビューを頼むのも気まずいので、自分から積極的にレビューを手伝って、見返りとして自分のコードのレビューもしてもらう」といったレビューの循環が回っています。
単純に順番制や当番制でも問題ないとは思いますが、自主性を上手く仕組み化できるようなやり方を考えてみるのも面白いと思います。
また私たちの例になりますが、ツールを利用してコードレビューの依頼自体を自動化したり、毎週コードレビューをした数に応じてランキングを作るなどのゲーミフィケーションを採用したりといったことで、コードレビューへのモチベーションを高めています。
チャットツールをGitHub連携してレビュー依頼を自動化
毎週レビュー数のランキングを掲載し、レビュアーのモチベーションアップに繋げる
ここまでのまとめ
この章ではコードレビューを始める前に決めておくとスムーズになる、4つのポイントを考えてみました。
- コードレビューの目的、観点を共有する
- 目的に応じてコードレビューをどのタイミングで行うのかを決める
- コードレビューを行う単位を決める
- コードレビューの担当の持ち回り方を決める
中でも「目的、観点の共有」は非常に大事なことなので、ぜひチームで話し合う機会を作ってください。
コードレビューを円滑にするコツ
コードレビューを始められる準備が整ったので、コードレビューでのコミュニケーションをより円滑にするためのコツもお伝えします。
エンジニアにとって、ソースコードとは心血注いで創り出したもの、いわば作品だと言ってもいいものですよね。そんな作品に対してコメントするのがコードレビューです。アプリをストアに出している人は分かるかもしれませんが、いくら本当のことだと言っても心ないレビューコメントをされると、とても傷つきます。
コードレビューでも同じです。どんな一言が相手を傷つけるか分かりません。ネット上を見ても、コードレビューを起点とした様々なトラブルが見られます。
コードに対するコメントというのはそれほど繊細なものなので、コードレビューにおいては、あらかじめいくつかの前提事項を共有できていると、不要なトラブルを避けられるでしょう。本章ではトラブルを避け、さらにレビューのコメントを有意義にするためのいくつかのコツをご紹介します。
コードに対する指摘は人格否定でないことを理解すること
レビューアから見てひどいコードに見えたとしても、人格を否定するようなコメントを書くのはそもそも良くないことですが、レビューイから見ると単純なコードに対する指摘すらも自身を否定されているかのように感じることがあります。
コードレビューは仕事の出来不出来について話す場ではなく、コードをどのように改善できるか話す場なので、
- コメントする側はコードを書いた人ではなくコードに対してコメントをする
- コメントされた側は自分に対する指摘ではなくコードに対する指摘と受け止める
- コメントする側は人格否定と捉えかねないようなコメントをしないこと
という前提を共有するといいでしょう。こういったコードレビューにおけるコミュニケーションに関しては、以下の記事に解説を譲ります。
意図が分からないコードについては「質問」する
レビューをしていると、いろいろ疑問に思うことがあるでしょう。
- ここでどうしてこの処理を挟んでいるのだろう？
- この例外処理は理由があってしているのだろうか？
- なんでこういう書き方をしているんだろう？
コードを読んでいて意図が分からなくなった場合は「なんでこういう書き方をしているの？」と質問してみると、お互いのコードに対する考えが深まります。
レビューイからすると「なんでこの質問をされているんだろう？ コードだけだとこの意図が伝わらなかったのかな？」と考え直して、よりコードを読みやすくするためにはどうするべきか、と考えるきっかけになります。また、レビューアからすると単純に自分の力量不足で読み解けなかった場合もあるので新しい学びに発展する可能性があります。
特にレビューアが若手の場合は、どんどん質問してみるといいでしょう。全てのプルリクエストを読み切ってやろうという気持ちで挑めばコードリーディングの鍛錬にもなりますし、先輩からの教えを受けられる絶好のチャンスになります。
なぜ悪いコードなのかを論理的に説明すること
レビューで指摘するからにはなぜ悪いコードなのかを論理的に説明しましょう。一番悪いのが「このコード、ビミョー」とだけ書くようなケースです。相手からすれば悪口を言われているだけのように受け止められてしまいますよね。
逆に考えて論理的に説明できないのであれば、それは自身の力量がまだまだだという証なので、素直に「意図が分からないコードについては質問する」プラクティスで教えを乞うのが正しい姿でしょう。
我が身に返ることを恐れずに指摘すること
例えば差分で新たに発生した運用上の前提条件をREADMEに書いておいてほしい、なんて指摘をしようと思ったときに、「いや、自分はできていないから、言いづらいな……」と思うことがあるかもしれません。とはいえ、誰もが自分ができていることしか指摘してはいけない、となってしまうと、全体的に萎縮してしまって改善が前向きに進みません。
レビューアになったからには自分のことは棚に上げて、あるべき姿に向けて指摘する。これが「我が身に返ることを恐れずに指摘する」プラクティスです。
「あー、自分もできていないな」と思ったら、後で自分が指摘した内容を自分のコードでも直せばいいのです。コードベース全体で考えれば相手のコードも直って自分のコードも直る、という2つの効果がありますよね。
良いところもコメントすること
コードレビューといえば悪いところばかりをコメントで指摘するようなイメージがあります。コードの改善だけ考えればその運用でも問題ないかもしれませんが、チームの雰囲気は悪くなってしまいますよね……。むしろレビューアとしては、コードの差分から学びを得る気持ちで、良いと思った部分についてもコメントするとコードレビューの雰囲気が良くなるのでオススメです。
「我が身に返ることを恐れずに指摘すること」で自分のことを棚に上げる例について触れましたが、逆に自分がやっていないような気配りを、他の人のコードの差分から発見することもあるかもしれません。そんなときは「おー、よくやってるな」と心の中で想うだけでなく、「これ、いいね！」とコメントしてみると、レビューされている側としては意外と嬉しいものです。
コードレビューはお互いのソースコードという作品に対して評価し合う絶好の機会です。この機会を活用してコードを介したコミュニケーションを行うことで、会社のエンジニア文化はより成熟していくのではないかと思います。
指摘は素直な気持ちで受け入れること
コードレビューはコードを改善していくことが目的の一つなのですから、受けた指摘を素直な気持ちで受け入れる心づもりで臨みましょう。指摘をしても「そういう考え方もあるよね」と、指摘をスルーする人がいますが、スルーしてばかりでは改善が回らなくなってしまいますね。
なぜスルーが発生するかにも目を向けましょう。「良いコード」について共通認識が持てていないことが原因なら、良いコードについてみんなで話し合う機会を設けてみるといいでしょう。
絵文字は積極的に使おう
文字でのコミュニケーションは油断すると淡白になりがちです。ちょっとしたことかもしれませんが、絵文字を積極的に使うと印象はだいぶ柔らかくなります。
コードレビューは油断するとトゲトゲしい言葉の応酬になりがちな側面もあるので、絵文字を活用して表現を丸くしてみるのも、円滑なコミュニケーションを維持する一つのコツだと思います。
設計レベルの指摘はコメントだけでやり取りするのではなく対面で話し合おう
さて、コードレビューはプルリクエストにコメントするのが基本ですが、ときにプルリクエストへのコメントだけでは収まらないような指摘をしなくてはいけない場合もあります。
そもそものクラス設計やモデル設計がおかしいのではないか？ という場合がそうですね。そんな場合は早々に文字ベースのレビューを打ち切って、コードを見ながら対面で話すのがスムーズです。設計レベルの指摘になるとコメントで表現するのは難しいですし、頑張って書いても相手が理解できない可能性もあるので、ただただレビューの時間だけが過ぎていってしまう可能性があります。
対面と言ってもリアルで対面する必要はなく、Googleハングアウトやzoom.usといった画面共有可能なオンライン会議ツールで、サクッと画面共有しながら設計意図を確認するのがスムーズでしょう。この際、話した内容についてリアルタイムにプルリクエストのコメントに書き込んでいくと議事録代わりになって、他のメンバーへの共有にもなるので一石二鳥です。
対面で話し、スムーズかつ確実にレビューを進める
ここまでのまとめ
コードレビューのコツを改めてまとめると以下になります。
- コードに対する指摘は人格否定でないと理解すること
- 意図が分からないコードについては「質問」する
- なぜ悪いコードなのかを論理的に説明すること
- 我が身に返ることを恐れずに指摘すること
- 良いところもコメントすること
- 指摘は素直な気持ちで受け入れること
- 絵文字は積極的に使おう
- 設計レベルの指摘はコメントだけでやり取りするのではなく対面で話し合おう
また、これらのコードレビューにおける重要ポイントに当たって、以前筆者はデキるプログラマだけが知っているコードレビュー7つの秘訣という内容で、以下のように7つにまとめたことがあります。
- レビューの観点を明確にすること
- 我が身に返ることを恐れずに指摘すること
- なぜ悪いコードなのかを論理的に説明すること
- 良いコードについて共通認識を持つこと
- 小さい単位でレビューを繰り返すこと
- 指摘は素直な気持ちで受け入れること
- 指摘は人格否定でないことを理解すること
本章の内容は基本的には上記の内容を踏まえたものになりますが、あわせてご参考ください。
総括
本稿では以下の内容についてご紹介しました。
- コードレビューで解決する問題とは何か？
- コードレビューを始める前に何を準備するべきか？
- コードレビューを回す上でのコツは何か？
ソフトウェアの中心はソースコードであり、ソフトウェアひいてはソースコードを健全に保つためには継続した改善活動を行う必要があります。ただ、一言で改善活動と言っても、ソースコードはエンジニアの作品の集まりであり、杓子定規に決まりを作れば改善が進むようなものでもありません。チームの成長がソフトウェアの品質に直結します。
中でもコードレビューはチームの成長を促す絶好の機会だと筆者は考えています。ぜひ本稿の内容を参考に、素晴らしいコードレビューの文化を作り上げて頂ければと思います。
著者プロフィール
西見 公宏（にしみ・まさひろ）@mah_lab
ブログ：mah365 | Webテクノロジーやビジネスに関わるアレコレ
*1:ソースコードの経路の複雑度を示す
