ヤフー株式会社は、2023年10月1日にLINEヤフー株式会社になりました。LINEヤフー株式会社の新しいブログはこちらです。LINEヤフー Tech Blog

テクノロジー

Cognitive Complexityを400以上減らすまでに何をしたか 〜 コード品質改善の具体的なプラクティス

こんにちは。Yahoo!広告 ディスプレイ広告エンジニアの安田です。私たちの開発チームでは広告配信の起点となるJavaScript(TypeScript)ライブラリを提供しています。今回はこのライブラリのデプロイ失敗率を改善できた、コード品質改善の取り組みについてご紹介します。

コード品質を定量的に測る指標の1つにCognitive Complexityがあります。Cognitive Complexityは人間視点での複雑性を評価する指標で、例えばネストが深くなるほど複雑と判断される特徴があります。複雑なコードは変更に多くの時間を要し、テストが難しくなるので要改善なシグナルといえるでしょう。私たちが今回実施した品質改善の取り組みでは、コードの状態を定量的に説明するための数値の1つとしてCognitive Complexityを採用していました。実際にコード品質を向上させるまでに取り組みを紹介しつつ、Cognitive Complexityの値が低下(改善)した背景を解説します。

Change Failure Rateが課題だった

デプロイした回数に対して失敗した回数の割合を示すCFR(Change Failure Rate)という値があります。改善する以前のライブラリのCFRは比較的高い値で、頻発する事故対応に工数がかかりデプロイに対するモチベーションが低下しかねない事態でした。さらに、コードが難解なことで機能追加ごとに費やすコードリーディングの時間が長く、開発開始から本番デプロイまでの時間を示すCLT(Change Lead Time)も芳しい状況とはいえませんでした。このような背景からコード品質を改善すべきという雰囲気があり、まず定性的に分析してみました。

  • 1万LOC規模のコードにもかかわらず主要な機能がたった2つのファイルに記述されている
  • 状態を複数のグローバル変数で管理している
  • 1つの変数を書き換えるだけでも生じる影響範囲が大きく、機能追加のたびに多くのテストコードを追記している
  • ユニットテストのカバレッジは一見高かったが、いくつかの重要なシナリオがテストできていない

コード設計を見直し品質向上を図ることは決まりましたが、現在の品質と改善後の品質を定量的に分析できなければ、その改善の良し悪しは判断できません。いくつかの定量化できそうな指標を模索した中で結果的に重視したのはCognitive Complexityでした。コードの複雑性は計測しやすく、複雑になればコードを理解しにくくなるのは自明であることからコードの品質を定量的に説明しやすい指標でしょう。

Cognitive Complexityが下がるまでにやったこと

1. Cognitive Complexityの目標値を設定する

Cognitive Complexityの適切な値についてはっきりとしたものはありませんが、コード品質を計測するプラットフォームであるSonarQubeでは1メソッドあたり15以下に、同様にCode Climateでは1メソッドあたり5以下にするように指摘されます。前述した主要な機能が記述されたファイルについて計測したところCognitive Complexityは291でした。1ファイルにメソッドが10個存在したとしても、1ファイルで300近い数値というのはあまりにも大きいといえるでしょう。品質改善後もなお高い数値であったのならば、その改善はうまくいっていない可能性が高いです。

私たちのチームでは1ファイルあたりのCognitive Complexityが50以下になっているかをチェックしました。50以下という数値に一般的な論拠はありませんが、前述した品質1メソッドあたりの1ファイルあたりのメソッド数やコード量を参考に採用してみました。一方で、ライブラリ全体のCognitive Complexityについては特に目標を設けませんでした。モジュールが適切に分割されファイルに配置されていれば、1つの機能変更によって多くのファイルを参照する必要はないからです。

Cognitive Complexityと同様に複雑度を計測する指標にCyclomatic Complexityがあります。それぞれのイメージを説明するために次の2つのコードを例に挙げます。

Cyclomatic Complexityは同一だが、Cognitive Complexityが異なる書き方の比較

これは極端な例ですが、ほとんどの人がmethodAをメンテナンスしたいと感じるでしょう。methodAはCognitive Complexityが1、Cyclomatic Complexityが4のメソッドで、methodBはそれぞれCognitive Complexityが6、Cyclomatic Complexityが4のメソッドです。どちらも同一の処理内容で、Cyclomatic Complexityも同一ですが、Cognitive Complexityが異なります。同じ処理を実現するコードでも理解しやすさは書き方次第で大きく異なることが分かるでしょう。Cyclomatic Complexityが下がると分岐網羅するために必要なテストケースを減らせますが、これを下げられるかは要件に依存する部分が大きいでしょう。また、先に挙げた例からも分かる通り、理解しやすさが指標に加味されることからCyclomatic ComplexityよりもCognitive Complexityを重視しました。

2. 機能を削除・統一して、ステートレス、疎結合、高凝集になるよう実装を見直す

目標を決めた後はいよいよコード実装を再設計して改善を図りました。仕様を整理して機能を削除・統一し、ステートレス、疎結合、高凝集に書き直すことを念頭に置きました。

仕様の整理にあたって実施したことは、ライブラリの使用状況の計測です。次のようにあるメソッドがどのような状況で実行されたか、ライブラリ全体でどのようなエラーが発生したのかをレポートします。ユーザーのデータ送信量を抑制するため、一定確率で送信するようにサンプリングしています。

// メソッドの使われ方をレポート
function f(args) {
  sendReport(args);
  // do something
}

// ライブラリ全体で発生したエラーをレポート
try {
  main();
} catch (err) {
  sendReport(err);
}

収集したレポートから必要ない機能を発見したり、機能の使われ方からよりまとまった仕様へ変更するヒントを得られます。私たちの場合ですと、あるモジュールでは10%程度不要な機能を削除でき、3つに分かれていたインターフェースを1つにまとめることができました。機能の削除や統一などで複雑度が減少すれば、実装やテストはシンプルになり、デプロイに失敗する確率は減少します。

コードの実装においては、初めに状態管理を見直して多くのモジュールがステートレスになるように修正しました。ステートレスではないモジュールはテストが難しいです。私たちが課題としていた「いくつかの重要なシナリオがテストできていない」という点に対して最も有効にアプローチできたのが、このステートレスへモジュールを書き換えることでした。ステートレスに書き換えるにあたっても先に述べたレポートは役立ちました。具体的には配信すべき広告の種別と実行環境に応じて複数の状態が存在し、それぞれの状態遷移で実施すべき事柄が異なっていた点をできる限り抽象化するためのヒントを得られました。疎結合、高凝集についてはレビューにてチェックしました。後述の節で説明しますが、これらについては定量的に計測する指標が見つかっていません。

上記の方法でコードを見直した段階で、モジュールは適切に分割され複雑度は目標とする値まで下がっていました。さらにCognitive Complexityを減少させるためには、アーリーリターン、メソッドチェイン、言語特有のUtilityを活用することが挙げられます。

3. 開発サイクルを高速化する

コーディングからデプロイまでの開発サイクルの長期化は品質へ悪影響をもたらします。例えば、ブランチが長期間残存することで意図しない変更が入り込んだり、実装者の記憶が薄れてミスが増えてしまうことなどが挙げられます。また、コード品質を改善すると思っていた変更が逆に悪化させる結果だったという事態の把握も遅れがちです。特にコードレビューは複数のレビューイからレビューを受けることがあるので開発サイクル長期化の要因の1つとなりえます。
そこで、設計やコーディングをしていない工程をできる限り短くするために、1つのPull Requestで変更可能なコード行数を制限することにしました。これにより、レビューイが要する説明量とレビュアーがレビューにかける時間が減るので、チーム全体としてコーディングから離れる1回あたりの時間を短縮できました。さらに同時にレビューの精度を向上することも期待できます。コードのレビューは複数の観点でリリース可能なコミットであるかを判断する必要があり難しいです。レビューすべき変更量が減れば確認すべき量も制限されるので見落としが減り、品質の向上に貢献することでしょう。(参考:https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/

4. 実装ガイドラインの作成

品質を改善してもそれを保つ仕組みがなければ、機能追加のたびに徐々に品質が低下します。さらに、チームでは複数のシステムを管理していて、誰もがいつでもこのライブラリに詳しい状況ではありません。モジュールの設計方針やテストの実装方針などコーディングに関するものはすべて確認できる状態が望ましいです。そこで実装ガイドラインとして、用語集や実装方針、モジュール設計、言語特有で注意すべきこと、テストの書き方などをまとめています。

また、どう実装すべきかだけではなく、なぜそのようにするのかの決定背景を必ず残すようにしています。今回の品質改善で再設計した際は、なぜそのような実装になっているのかがコードやコメントから読み取れず、変更可能なのかがすぐに理解できないことがありました。現在はADR(Architectural Decision Records)を作成して決定に至った背景を検索できるようにしています。

変更可能なコード量を超過したときに警告される

コード品質改善によって得られた効果

前述したプラクティスに従って、仕様の整理や実装、レビュー、リリースを並列して実施しました。リリースにあたっては計測しているライブラリの使用状況のレポートから、ユーザーに悪影響を及ぼしていないかを確認しながら進めました。次にコード品質改善前後のCognitive Complexityの変化について示します。

コード品質改善前後のCognitive Complexityの変化

Cognitive Complexityはリリースごとに徐々に改善され、最終的に全体で400以上減少し1ファイルあたり20前後の数値となりました。改善前後の10カ月間のCFRを比較したところ7.2ポイントほど低減し、コード品質改善によるポジティブな結果が得られました。

また、副次的にいくつかの良い影響がありました。

  • 改善対象に関連するシステムのI/Fを見直すきっかけが生まれ、結果として他システムの処理がシンプルに改善された
  • CIの実行が高速になったことでリリースやロールバックに必要な時間が短縮された

さらに、開発に携わる私たち自身が良いコードになったと実感でき開発モチベーションを向上できた点は効果が大きかったです。品質改善目標を達成した後にチームで振り返りした際は、このような感想が挙がっていました。

品質改善目標を達成した後にチームで振り返りしたときのボード

今後の課題

単にCognitive Complexityを下げるだけでしたら、メソッドやファイルを分割するだけで達成できてしまいます。必要以上にメソッドをバラバラに配置することは可読性を下げるので、コード品質が高い状態とはいえないでしょう。今回の品質改善ではモジュールを再設計し、そのメソッドやクラスが適切な場所に配置されているのかはレビューで判断していました。モジュールやメソッドが適切に配置されているかを定量的に分析できておらず、この点は課題として残っています。

一般に良いコードとして知られる指標に「疎結合」、「高凝集」というものがあります。当初はこれらの指標を定量的に分析できないか試行錯誤し、結合度はグローバル参照可能な変数の数から、凝集度はLCOM4からの推定を試みていました。結果的にこれらの数値からの結合度や凝集度の判断は困難であることが分かり、現在は計測を続けていません。LCOM4については「コード品質を上げるために凝集度解析ツールをGo言語で自作した話」にて考察されていますので、興味がありましたらご参考ください。

その他の課題として、ファイルサイズを品質の指標として用いていないことが挙げられます。冒頭に申し上げた通り、複雑なコードは改善すべきシグナルという理念で品質改善を実施したので、たとえコード量が増加したとしてもCognitive Complexityが下がる方を優先しています。サーバーで稼働する多くのシステムにおいてはこの手法で問題ないですが、フロントエンドで動作するライブラリについては考慮が必要と考えています。今回改善したライブラリは数万QPS規模で多くのユーザーから参照されるため、ファイルサイズの増加によるWeb Vitalsへの影響を検討しています。現在は、Pull Requestごとに最終的に出力されるファイルサイズが設定されたしきい値を超えないかをチェックしています。しきい値を超えない限りはコード量が増えても問題ないとしていますが、ユーザーにとって良い状態を評価し見直す可能性があります。

おわりに

Cognitive Complexityを下げるまでの過程と課題について説明しました。この記事で紹介した内容を次の図にまとめます。

コード品質改善の具体的なプラクティスの鳥瞰図

1点追加で補足しますと、Cognitive Complexityを下げること自体に注力してはいけません。コード品質を測る指標の1つとしてCognitive Complexityは使えそうですが、Cognitive Complexityを下げたからといってコード品質が必ずしも高いとはいえないでしょう。モジュールの分割が適切な状況で初めてCognitive Complexityを下げることに意味があり、コード品質改善には説明したようないくつかのステップが必要です。本記事からコード品質改善に取り組む際のヒントが見つかりましたら幸いです。

こちらの記事のご感想を聞かせください。

  • 学びがある
  • わかりやすい
  • 新しい視点

ご感想ありがとうございました


安田 昂樹
Yahoo!広告エンジニア
広告配信システムの開発と運用を担当しています。SchemeやOCamlのような言語が私の理想ですが付き合った時間はJavaScriptが長く、最近はGoを書く時間が長いです。

このページの先頭へ