はじめに
多くのプログラマは保守性の低いコードに遭遇した時に、次のような一曲を歌ったことがあるでしょう。
はぁ〜 テスト（コード）も無エ
lint(静的解析)も無エ
コンパイル警告も それほど 取れて無エ
分岐は多い ネストも深い
回収時に目が ぐーるぐる
オラ こんな コードいやだ
オラ こんな コードいやだ
テストコード書くだ
テストコード書いたら 工数貯めて
リファクタリング するだ
このような、気持ちはわかりますが、実際、「オラ こんな コードいやだ」と言うのは難しいことです。
特に大規模プロジェクトの場合、様々な背景のメンバーが参入されており、一律の品質を確保するのは困難になります。
ある種の人間にとっては正しいコードが別の人間にとっては悪いコードとなることもありえます。
また、あるプロジェクトでは正しい書き方が、別のプロジェクトによっては悪い書き方となることがあります。
重要なのは、プロジェクトにおいて、一律のルールが守られている状態にすることです。
本稿では、「オラこんなコードいやだ」という言葉を、個人の『お気持ち』を排除して言うにはどうしたらいいかをを考えたものとなります。
また、この記事は初心者向けの記事ではありません。
そもそも初心者向けの記事というのは幅広い知識と正しい言語能力が備わっている人間によってのみ記述できるものであり、それは私の能力を超えるものであります。
全くの初心者の方は、このような個人のお気持ちを書いている怪しげなサイトを読むよりは、職場の先輩と相談した方が有益な結果になるかと思います。
この記事はレビュワーを突然ふられた方や、現在、絶賛レビュワー中の人にとって多少なりとものヒントになれば幸いという目的になっています。
静的解析をする
おそらく、多くのプログラミング言語では静的解析のツールが存在します。
有償のものから無料の物まで様々存在しますので、まずはそのご利用をお勧めします。
これらの静的解析を実行することにより、バグに繋がる危険な実装を容易に知ることができます。
人力でチェックして、お気持ちを表明するより、機械的に判定した方が正確で楽です。
静的解析ツールはJavaScriptであればeslint、JavaであればFindBugなどは無償でも強力なツールが存在しております。
VBAの静的解析の例
https://qiita.com/mima_ita/items/3629006163d0a7fa6e55
node.jsの静的解析の例
https://qiita.com/mima_ita/items/1cc6b2fb938d5a1fc3aa#%E9%9D%99%E7%9A%84%E8%A7%A3%E6%9E%90
C言語の例
https://qiita.com/mima_ita/items/cb94a767e9d9e5a84a2f#%E9%9D%99%E7%9A%84%E8%A7%A3%E6%9E%90
VisualStudioでのマネージドコードの解析例
https://docs.microsoft.com/ja-jp/visualstudio/code-quality/roslyn-analyzers-overview?view=vs-2019
メトリックスを計測する
ソースコードのメトリックスを計測するのは重要です。
メトリックスとして、よく計測される物には次のような物があります。
- ステップ数
- コメント率
- ネストの深さ
- 循環的複雑度
ステップ数
一昔前はステップ数で、支払額が増減する恐ろしい話もあった物ですが、現在においても規模をみるのには有効な数値であると考えられます。
例えば、１クラスに大規模なステップ数があった場合、そのクラスを適切に分割する必要があるなどの情報をもたらします。
ほとんどのメトリックスを計測するツールはステップ数は計測している物が多いので、よく使用される指標になるでしょう。
ただし、ステップ数はあくまで規模の目安にすぎず、過度な期待はやめておいた方がよろしいです。
ステップ数あたりのバグ件数を求める現場は多くありますが、それはあくまで目安として運用すべきであり、絶対的なものとして扱わない方がいいでしょう。
コメント率
ステップ数あたりのコメントの割合を計測します。
多すぎても、少なすぎても、なんらかのイレギュラーな可能性があるので、目視で確認する必要が出てきます。
ネストの深さ
ネストの深さも又、可読性に関わるため、一部のメトリックス計測ツールでは計測されています。
if (a === 'neko') {
if (b === 'inu') {
if (c === 'tori') {
// 深いネスト 読み辛い
}
}
}
if (a === 'neko' && b === 'inu' && c === 'tori') {
// 浅いネスト 読みやすい
}
循環的複雑度
循環的複雑度は、一言で言うと分岐の多さで、分岐が多いほどテストが難しくなりバグが出やすくなると言う指標です。
function a() {
// 複雑度 1
console.log('hoge')
}
function b() {
// 複雑度 2
if (x) {
}
}
function c() {
// 複雑度 4
if (x) { // +1
} else if (y || z) { // +2
}
}
単純に、可読性が低いと言うよりも、これらの指標を使用してコードの品質に言及すべきでしょう。
なお、eslintやVisual Studioの静的解析などは循環的複雑度が高すぎる場合に警告を出力させることができます。
メトリックスの計測ツールの例
メトリックを計測するツールは様々あります。
Source MonitorはWindowsのアプリケーションで、C ++、C、C＃、VB.NET、Java、Delphi、Visual Basic（VB6）で書かれたソースコードのメトリクスを計測することが可能なフリーソフトです。
以前、詳細を記述したので今でも参考になるかもしれません。
https://qiita.com/mima_ita/items/cb94a767e9d9e5a84a2f#source-monitor
VisualStudioで開発している場合は、標準の機能として計測できます。
Pythonの場合は以前はRadonで計測しましたが、何年も前の話なので今は別の方法があると思います。
JavaScriptの場合、以前はPlatoで計測してましたが、vue.jsの開発時は計測ができず自作したことがあります。後述のSonarQubeも検討した方がいいでしょう。
上記のツールのようにローカルのPCにインストールして使用しますが、Webベースのツールも存在します。
SonarQubeは様々なプログラミング言語のメトリックスの計測と静的解析を行ってくれる強力なツールです。ソースコードの解析結果をサーバーに蓄積してWebページから閲覧することが可能です。
先人の方がdockerで起動する方法を紹介しているので、まずは試してみるのがよろしいかと思います。
SonerQubeを使ったGoプロジェクトの品質管理
https://qiita.com/nanamen/items/37ac8b05fdf5af7c239f
ただし、JavaScriptの循環的複雑度に関しては、関数単位でなくファイル単位でしか表示されないことに注意してください。又、SoanrQubeは循環的複雑度の他にCOGNITIVE COMPLEXITYという循環的複雑度を発展させた直感的に感じる複雑さを表す測定値を計測可能になっています。
さらに、コードの重複を測ってくれているので、コピペ防止にも役に立ちます。
SonerQube以外では以下のようなコピペ検出ツールが存在します。
https://qiita.com/mima_ita/items/fe1b3443a363e5d90a21
なお、AWSの一番安いEC2だと立ち上がらない程度には、マシンスペックを使うのでサーバーを調達する際は注意してください。
コーディングルール/ガイドラインを作る
レビューを行う際に、レビュワーによって基準がぶれてはいけません。
そのため、コーディングルール/ガイドラインを制定する必要があります。
静的解析やメトリックスの数値で機械的に計測できるもの以外のルールを制定していくことになります。
一般的なプログラミング言語についてのコーディングルールやベストプラクティスは既に誰かが制定している物があるのでそれを参考にしましょう。Effective C++,Effective C#, Effective Java, Effective Pythonなどの書籍は参考になります。（ただしバージョンには注意。書籍は基本的に古い）
これらを参考にしてルールを制定後、プロジェクト固有のルールを盛り込んでいくといいでしょう。
以前、VBAのガイドライン案を公開したことがあります。
Excel VBAコーディング ガイドライン案
https://qiita.com/mima_ita/items/8b0eec3b5a81f168822d
ここで一番重要なことは、冒頭で述べたことです。
ここで記述する内容はあくまでガイドライン、指針の案にすぎない。この規約を守るためにコードを作るのでなく、よいコードを作るためのガイドラインにすぎない。このガイドラインがよいコードを作るのに障害になる場合は、ガイドラインを変えるか、ガイドラインを使用しない。
つまり、このガイドラインは必要に応じて、変更されることがある
コーディングルールを決めた時には正しかったこと、あるいは正しいと思い込んでいたことが、後になって間違っていることなどよくあります。（上記の文章も実際嘘を書いていた）
重要なのは、コーディングルールそのものではなくて、そのコーディングルールを必要に応じて変更できるようにすることです。
レビューをする
さて、ここまできたらようやくレビューのプロセスが開始できます。
レビューについては以前、記載しました。
あなたのおっしゃるレビューってどのことかしら？
https://qiita.com/mima_ita/items/c3490ad1ccc12ad853f1
ここで最も重要で、最も難しいことは以下のことです。
どんな酷いコードにみえても、相応の苦労があったので、そこは敬意を払うべきです。
そして毎回、建設的で有意義な意見を述べるように努力してください。
特に、よく燃えているプロジェクトで、大量の新兵のレビューをするときなどは、この心得を忘れがちになってしましますが、決して忘れてはいけません。
そういう状況下であるとコンパイルすら通らないコードがレビューに流れてくることが、まれによくあります。
しかし、そこで怒ってもしょうがないですし、レビューの前段階でレビューイ本人が気付ける仕組みを整えることに注力した方が、余程、建設的になります。（二敗）
最後に
さて、ここまで読んでいただけたところで冒頭の目的を思い出して見てください。
「オラこんなコードいやだ」という言葉を、個人の『お気持ち』を排除して言うにはどうしたらいいかをを考えたものとなります。
タイトル詐欺となりますが、結論としては、そもそも、そのようなセリフは言う必要はなく、以下のように置き換わると思います。
- 静的解析で警告が出ているので直してください。例外の理由があるなら説明してください。
- 複雑度が基準以上でいているので直してください。例外の理由があるなら説明してください。
- コーディングルールと違うので直してください。例外の理由があるなら説明してください。あるいはルールが間違っていると思うなら根拠を教えてください。
- この実装にはこう言うリスクがあるように見えるので、単体テストで確認してください。
どうしても「オラこんなコードいやだ」と言いたい場合は、居酒屋で醤油差しに向かって言っていただくことをお勧めします。それは自身の気持ちを一時的にはらすと言う以上の効果はなく、状況の改善には役に立たないと私は思います。