こんにちは。Android版Yahoo!カーナビの開発を担当している根岸です。
Yahoo!カーナビではコード品質の可視化と改善に取り組んでいます。本記事ではなぜ品質の改善に着手することになったのか、その経緯から始まり、実際に可視化から改善まで行ったプロセスを紹介します。
Yahoo!カーナビの課題
Yahoo!カーナビは2014年7月31日に初回リリースし、来年でリリースから10年ほど経ちます。長く愛されているアプリですが、長さゆえに課題も多くあります。中でもコードの品質が低くなっているのが重要な課題でした。
コードの品質が低くなった一つの理由として、長い間運用してきたから、というのがありそうです。開発者の声を聞いたりソースコードを見ると、以下の問題点がありそうでした。
- 何年も前のコードがそのまま使われている
- AndroidであればJava、iOSであればObjective-Cで書かれているコードがある
- いわゆる神Managerクラス、Fat Activityが居る
- 複雑に条件が入り組んだ長いメソッドがある
中でも、「複雑に条件が入り組んだ凄く長いメソッドがある」のは大きな課題であり、エンジニアがコードを読み解くのも一苦労な箇所がいくつもありました。これらの課題は、長年運用しているアプリにおいて起こり得る話しであり、Yahoo!カーナビ特有の問題ではないかと思います。
コードの「品質」とは
品質に課題があることは分かりました。では、「品質」とは具体的には何を指すのでしょうか。
私が好きなエンジニアの一人、t_wadaさんのスライド、質とスピードでは、(レガシーコードからの脱却から引用して)品質は内部品質と外部品質の2つに分けられると紹介されています。
内部品質は結果ではなく原因
出典:質とスピード(2022春版、質疑応答用資料付き) / Quality and Speed 2022 Spring Edition(外部サイト)ソフトウェアの品質を外部指標で特徴づける人は多い。正しいことをする、バグがない、速い、などだ。だが、それらはより深い原因の症状にすぎない。本書で説明するソフトウェアの品質は内部品質である。内部品質を作り込んだ結果として、外部品質として定義される特性の実現に近づくことができる。内部品質は結果ではなく原因であり、良いソフトウェアが備えているべきものだ
出典:レガシーコードからの脱却(外部サイト)
品質の悪いアプリはバグが起きたり、動作が遅かったりしますが、その外部品質の悪さの原因は内部品質にあるということです。
- 外部品質・・・ソフトウェアの正確性、レスポンスの速さなど
- 内部品質・・・変更のしやすさ、理解のしやすさ、テストのしやすさなど
さらに、(一般的にスピードの代わりに犠牲にされてしまう)内部品質は、より具体的に言うと保守性を指すとも言及されています。
ではどのような内部品質を犠牲に捧げたのか
保守性(Maintainability)
Maintainability(保守性)を構成するもの
・解析性 ≒ 理解容易性
・修正性 ≒ 変更容易性
・試験性 ≒ テスト容易性
出典:質とスピード(2022春版、質疑応答用資料付き) / Quality and Speed 2022 Spring Edition(外部サイト)
冒頭で挙げた「複雑に条件が入り組んだ凄く長いメソッドがある」は、まさに保守性が低い状態であると考えました。
保守性を主とした内部品質を向上させることで、不具合の発生を抑止し、巡り巡ってサービスや会社の発展につながると考えました。(私たちのチームには、プロダクトの品質向上を支援する立場の方がいます。その方に書いて頂いたループ図を以下に載せます)
では、品質をどのように計測・表現するか
次に、内部品質を図る指標を定めました。品質につながる指標として、現実的に計測可能で、かつ効果のあるものが必要です。
- テストカバレッジ
- 循環的複雑度
- コードの行数
- Kotlin/Java、Swift/Objectiv-Cの割合
上記の指標が考えられますが、私たちは循環的複雑度に着目しました。循環的複雑度とは、コードの複雑性を表す指標であり、一般的には以下のように分類されます。
循環的複雑度 | 複雑さの状態 | バグ混入確率 |
---|---|---|
10以下 | 非常に良い構造 | 25% |
30以上 | 構造的なリスクあり | 40% |
50以上 | テスト不可能 | 70% |
75以上 | いかなる変更も誤修正を生む | 98% |
※循環的複雑度を改善する以外にも、テストカバレッジを向上させるためにコードを整理したり、テストコードを増やしたり、アーキテクチャを刷新するためにリファクタリングを進めたり、さまざまな活動をしていますが本記事では割愛します。
実際、Yahoo!カーナビのAndroid版/iOS版共に、複雑度が75以上のメソッドが存在しました。
循環的複雑度の可視化
指標が決まったので、次に着手するべきは指標の可視化です。可視化については以下の2点が重要だと考えました。
- 自動で計測・集計できる(もしくは単純な手動作業で計測できる)
- 誰でもすぐに見られる
複雑度を計測する
モバイルアプリにおいて、循環的複雑度を計測する方法はいくつかあります。私たちは同じZホールディングスグループのZOZOのTECH BLOG「ZOZOTOWN Androidチームにおけるコードメトリクスとビルド時間計測の取り組み」を参考にしました。
Androidにおいては、Javaはcheckstyle/checkstyle、Kotlinはdetekt/detektで計測しました。
checkstyle.xml
max
のvalue
で許容する複雑度の最大値を指定。0だと1以上の複雑度のメソッドを全て警告扱いとする。suppressions.xml
で計測対象から除外するファイルを指定(省略) <module name="CyclomaticComplexity"> <property name="max" value="0"/> </module> </module> <module name="SuppressionFilter"> <property name="file" value="./suppressions.xml"/> <property name="optional" value="false"/> </module> (省略)
suppressions.xml
suppress
で計測対象から除外するファイルを指定(省略) <suppressions> <suppress checks=".*" files="src/test/java/.*" /> <suppress checks=".*" files="src/androidTest/java/.*" /> <suppress checks=".*" files="build/generated/.*" /> (省略) </suppressions>
detekt.yml
ComplexMethod
のactive
をtrue
にして循環的複雑度の計測機能をONにするthreshold
で許容する複雑度の最大値を指定excludes
で計測対象から除外するファイルを指定ComplexMethod: active: true threshold: 0 (省略) excludes: ['**/test/**', '**/androidTest/**', '**/**/XXXX.kt', '**/ZZZZ.kt']
checkstyleとdetektで計測する理由としては以下の点になります。
- checkstyleはAndroid Studio用のプラグインがあり、開発中にローカルですぐに計測が可能(detektもローカルで実行できますが、出力されるxmlファイルを確認する必要あり)
- checkstyleとdetektをCI上で動かすノウハウが社内に十分あった
特にAndroid Studio上で複雑度を確認できるのは大きく、開発中の意識付けに効果的です。
- メソッドにカーソルをあてると複雑度が見える
- ローカル環境で複雑度が一定以上のファイル、メソッドを一覧で確認できる
また、現在は複雑度の自動計測や改善はAndroidでのみ行っています。(iOSも将来的に取り組む予定です)
誰でも見られるようにする
循環的複雑度を計測できるようになりましたが、その計測結果はXMLやHTMLに保存されます。結果を個別のファイルを開いて確認するのは面倒ですし、何より文字だけでは分かりづらいです。
そこで、計測した結果を統合し確認可能な内製のダッシュボードを作成しました。Reactなどのフレームワークを活用し、なるべくライトに作成しています。
ダッシュボードには以下の機能があります。
- checkstyle、detektの出力結果ファイルを解析し統合する
- 循環的複雑度の階級ごとにメソッド数を集計する
- ある時点において、全メソッドの複雑度の一覧が見られる
- ある時点の任意のメソッドのソースコードへのリンクをたどれる
- 過去から現在まで、各メトリクスの推移をグラフで確認可能
特に、グラフで複雑度の推移を見られる機能は重宝していて、エンジニア以外のメンバーと会話する際にも役立ちます。
循環的複雑度を改善する
いよいよ実際に複雑度を下げるための活動を行います。
改善活動の指針
複雑度を下げる簡単かつ効果的な方法は、メソッドの抽出/分割です。書籍「リファクタリング 既存のコードを安全に改善する」の中でも「関数の抽出」として極めて頻繁に行われるリファクタリングとして解説されています。
「関数の抽出」はきわめて頻繁に行われるリファクタリングです。(中略)コードの断片を見て、何をしているのか理解した上で、独立した関数として抽出し、目的にふさわしい名前を付けます。
出典:リファクタリング 既存のコードを安全に改善する(外部サイト)
IDEの機能でも抽出できますし、人の目で見て抽出もできます。ただ、メソッドを抽出して複雑度を下げるだけでは、効果的なリファクタリングとは言えません。複雑度という指標に現れない観点でも、より良いコードにするために、以下の点に注意してリファクタリングを行いました。
- なるべくKotlin/Swiftにコードを変更する
- 条件/メソッドの単純化
- 重複コードの共通化
- 不要コードを削除する
- ユニットテストが書きやすいメソッドにする
- 大きなメソッドの中で意味のある処理を抽出し、メソッド分割する
- 分割後のメソッドを意図に沿って命名し、処理の中身を想像しやすくする
また、「複雑度の改善をどのメソッドから行うか」についても定義しました。端的に言えば「変更頻度が高く、リファクタリングによる不具合発生リスクの少ない箇所」を優先的にリファクタリングしました。リファクタリングは、将来同じ箇所のコードを触る時に、理解しやすく変更しやすい状態になっていることで効果を発揮します。変更頻度が高い箇所は、将来的にもまた変更する箇所である可能性が高い、コア機能であると考えています。
複雑度が上がらないように監視する
リファクタリングを実施しているときは「循環的複雑度を上げない方がいい」と分かっていても、普段の機能開発時は意識しなくなってしまうこともあります。特に、既存のメソッドの中にそのまま処理を追記することで、意図せず複雑度を上げてしまうこともあります。
そこで、PullRequest(以下PR)を出した際に、CI上で循環的複雑度を計測し、一定の複雑度のメソッドを一覧表示する機構も用意しました。
これにより、意図せず複雑度が閾値を超えるようなことがなくなりました。また、PRを出した際にレビュアー/レビューイ共に複雑度が上がっていないか簡単に確認することができます。
改善サイクルを回す
これまで紹介した活動で、以下の改善サイクルができ上がりました。
- 既存のコードの複雑度が上がらないように監視し
- リファクタリングを行い
- 循環的複雑度を下げ
- 定期的に指標を監視し
- 次のアクションを決定する
この改善プロセス自体も何度か見直し、細かい部分で改善方法をチューニングしました。
※まずはAndroid版のみで改善しました。将来的にiOS版でも取り組む予定です。
改善の結果
改善の結果、Android版において、複雑度30以上のメソッド数、50以上のメソッド数、どちらも約半分に減らすことができました。
他にも以下の良い効果がありました。
- Kotlin率の向上
- 不要なコードの削除
- より良いコードを書くことへの意識の向上
- エンジニアが良いメソッドの分割方法について考えられた
- 他の職種のリファクタリングへの理解がより得られた
おわりに
本記事では、モバイルアプリの品質の改善に取り組んだプロセスを紹介しました。改善の結果、複雑度が高いメソッド数を約半減することができました。加えて、定性的ですが良いコードへのリファクタリングが進んでいる実感がありますし、エンジニアの意識も向上することができました。今後は、リアーキテクチャや自動テストの拡充など、多方面でコード改善にも取り組んでいきたいと思います。
コード品質の可視化に興味を持たれたかたは、Yahoo! JAPAN Tech Blogで他の指標も紹介されているので、よろしければご覧ください。
こちらの記事のご感想を聞かせください。
- 学びがある
- わかりやすい
- 新しい視点
ご感想ありがとうございました
- 根岸 拓郎
- Yahoo!カーナビ Androidエンジニア
- Android版Yahoo!カーナビとYahoo! MAPで車ナビ機能を担当しています。Yahoo!カーナビの開発効率向上プロジェクトも担当しています。