こんにちは。テクノロジー部テクニカルディレクターのさらです。
今回は、短い時間のなかでも効果的・効率的にソースコードレビューを行うために私が実践していることを書いていきたいと思います。
レビューのやり方を知らなかったころは1行ずつ読む、という超非生産的なことをしていました。今思えばただ時間を食うだけでなく、エラーも検知しにくい最悪なやり方だと思います。
この記事では、短い時間でも適切なコードレビューをするために心がけていることをご紹介できればと思います。
目次
コードレビューのポイントを押さえる
適切なコードレビューを行うためには、まず見るべきポイントを押さえる必要があります。私の場合、色々な要素を検討して試行錯誤した結果、下記のような分類に落ち着きました。
- 見た目の美しさ（インデントやタブ、改行が適切か）
- 動き（仕様通りか、負荷が大きくないか）
- 可読性（名付けは適切か、処理や制御がわかりやすい構造になっているか、ネストしすぎていないか）
- 可変性（拡張性）・保守性（適宜関数化ができているか、再利用がしやすいか）
これは、コードが帰属するプロジェクトやその方針にも左右されることと思いますので、コーディング規約などと照らし合わせながら自分の基準を作っていくのが理想かと思います。
基準を設けておくことで、どこに気をつければ良いか・コードがどうあるべきかがわかるため、効果的なコードレビューに繋がります。
目の大きいふるいから細かいふるいにかけていく
1.まず動かしてみる
これは純粋に読む作業ではないのでソースコードレビューの作業の一部としてしまうには不適切かもしれませんが、そもそもここで動きが変なところがあれば、漫然と読むよりもコードのどこに不備があるかのあたりをつけることができます。
2.見た目と名付けを確認する
特に前項で言及した見た目の美しさと可読性（名付けは適切か）の部分です。コメントが適切になされているか、わかりやすいかどうかもここで確認したいところです。一目でわかる部分なので、コードの内容を読み込む前にざっとスキャンするイメージです。
こちらの項目はコーディング規約が定められているプロジェクトでは基準があることが多いので、そちらと合わせて確認するのがよいかと思います。規約がない場合や、規約と併せて変数名の良し悪しを判断する基準に関しては後述します。
3.処理の内容・制御フローを確認する
ここで確認するのは、要件や設計通りの処理になっているか、適宜関数化されているか、冗長でないか（ネストしすぎていないか）、制御文がわかりやすいか（制御文のわかりやすさについても後述します）、変更・再利用がしやすいかなどです。処理や制御の内容を確認したあと、コメントがされていればその内容が正確かどうかも判断します。
1から3にかけて、目の大きいふるいから細かいふるいにかけていくイメージです。不備が検知しやすい順で、複数回に分けて見るため精度も上がります。
可読性が高いコードとは？
さて、コードレビューの目的はコードをより良いものにしていくことだと思いますが、まず前提として良いコードとは何か、考える必要があります。
こちらのプログラマーのバイブル・リーダブルコードでは、「理解しやすいコードこそが優れたコード」に繋がるとされています。そこで、理解しやすいコードの条件をリーダブルコードからまとめてみました。
変数名 = 短いコメントのようなもの
汎用的な名前でなく具体的で正確な名前をつけることで、その変数が何かを理解する時間の短縮に繋がります。例えばmakeなどの曖昧な動詞でなく、generateやcomposeなどの具体的な動詞を使う、retvalなどではなく戻り値にどんな値が入っているかを表す名前をつける、というような感じです。
また、基本的には一度しか使われないような値を変数に代入する必要はないですよね。ですが、パッと見てそれがわからないような値はあえてわかりやすい名前をつけて変数に入れることで、「これはこんな値なんだな」と一眼で判断することができます。説明変数・要約変数と呼ばれるものです。
行数を短くするよりも他人が理解するのにかかる時間を短くする
制御文では失敗フローから早めに返すことで、ネストが深くなるのを避けられます。改修で条件を加えるとき、脊髄反射で既存の条件文内に新たな条件文を加えそうになったら、既存の条件文の条件部分に足せるかどうか検討したいところです（なんだか音読したら舌を噛みそうな文になってしまいました）。
変数名や関数名ですが、短いに越したことはないと思ってなるべく短い名前をつけてしまうと、かえってわかりづらくなってしまうことがあります。スコープが大きい場合は、長い名前でも問題ありません。
逆を言えば、上記の「汎用的な名前を避ける」と矛盾するようですが、スコープが小さい場合・スクロールをせずに処理が完結するような場合は短い名前でも大丈夫です（パッと見てわかるということが最重要です！）
制御文のわかりやすさ
条件文の条件はド・モルガンの法則を活用し、わかりやすく書き換えましょう。肯定形を使うほうが理解にかかる時間は少なく、否定の否定は混乱を招くため避けるのが吉です。また、条件は基本的に話し言葉（日本語・英語）の文法と同じで左から右へ記述します。
例えば値が0かどうかを判定する条件ならば、if (val==0)のように書きます。
私は新卒のころ先輩に「定数は条件式の左辺に置きなさい」と教わったのですが、これはヨーダ記法と呼ばれるもので、バリデーションの==を=と書き間違えてしまった場合でも、シンタックスエラーとして検知されるようにする記法です。
ただ、現代のコンパイラはif文の中に代入文が入っていれば警告を出してくれるらしいので、可読性を重視すると通常通り左から右へ書くのがベストかと思います。
特に重要だと感じているものをピックアップしてみましたが、駆け出しエンジニアや駆け出しコードレビュアーの皆さまは、ぜひ本家を読んでみることをおすすめします。
おわりに
国際規格であるISO/IEC 9126では、ソフトウェアの品質を評価する品質特性モデルを機能性・信頼性・使用性・効率性・保守性・移植性の特性群にわけて規定しています。しかし、そのうち保守性・移植性に関してはテストを実施するだけで測ることは難しいため、ソースコードレビューを明確な基準を持って行うことが大切になってきます。
今回この記事を書くにあたって拝読させていただいたこちらの記事（2020年公開）に、IPAのセキュアプログラミング講座にて「ソースコードレビューは下読み・成熟度点検・機能点検・脆弱性点検の4つの段階に分けて行う」ことを推奨されていることが書かれていましたが、現在のセキュアプログラミング講座の内容を見るとこちらの記載は消えており、まずはソースコードの静的検査ツールやCIツールを活用することが推奨されていました。
今回の記事では手動でのレビューのことを書きましたが、自動化の波にも乗りたいですね🧐 CIツールも勉強中なので、いずれその記事も書けたらと思います！ これからも一緒に爆速で成長していきましょう〜〜〜👦🏻
LIGはWebサイト制作を支援しています。ご興味のある方は事業ぺージをぜひご覧ください。
