バグが多発する箇所の循環的複雑度を測ってみたら高すぎたので修正した話

BAN Masanobu
スタディスト Tech Blog
4 min readNov 1, 2019

--

スタディスト開発部です。平日ボルダリングをしているのですが、最近社内の人があまり来なくて寂しいです。社内、社外問わずに登りたい人は是非声をかけてください。

背景 どうしたの?

UIリニューアルRailsアップグレードにより、大分複雑なコードを整理しリファクタリングを進めてきました。しかし、いくら進めても終わりが見えて来ないのがリファクタリングです。

今回、リニューアルでも手をつけられなかったコード(バグが多発していた箇所)を、機能追加変更に伴いリファクタリング、修正をしていくことになりました。

どれくらいやばいの?

今回修正するコードをgemを使って循環的複雑度測ってみました。全ソースコードの中でも一番悪い50という複雑度指数でした。

どれくらいマズイのか見てみると循環的複雑度50というと「テスト不可能」バグ混入確率が70%にもなるということでした。バグ混入確率70%!通りでバグが多発するわけです。

なんかヤバそうです、手を付けたくありません、見たくもありません。でも手を付けざるを得ません。覚悟を決めてリファクタリングしていくことにしました。

どうやったの?

やったことは、現在の仕様を担保するために、まずは全体計画をたてて、下記のステップで改善を進めていきました。

ステップ1

まず、テストケースの洗い出しを行いました。もちろんそのコードには、テストは一切ありませんでした。「テスト不可能」ということですが、とりあえず正常系のパターンだけ洗い出し、そこを通るテストを書きました。異常系は、この分岐通るケースあるのだろうかという意味不明な箇所は除いて主なエラーパターンを網羅したテストを書きました。ただ、正しい仕様というのは、文章に残っていることも無く、不明なことが多かったので全ての分岐を通るテストを書くことは出来ませんでした。

ステップ2

次は、使われていない処理の削除です。こればっかりは、過去の経緯を知らなければ分からないので、長く在籍しているメンバーに確認して消しまくりました。キャンペー○コード?○○ユーザ?もう使われてない機能が多くありましたので、問答無用で削除しました。

ステップ3

今度は、メソッドの共通化です。当たり前ですね、同じ処理を何度も書いてる箇所がありましたので、メソッドを抜き出し、使い回すようにしました。だんだんスッキリしてきました。

後は、機能追加修正もしなくてはならず、リファクタリングにかけられる時間も限られていたので、循環的複雑度をどのあたりまで下げるか目標値を設けてそこまで下げることで今回のリファクタリング終わりとしました。(結果は次の章で)

結果

リファクタリングを行なった結果、1メソッド400行ぐらいあったのを100行未満に減らし、循環的複雑度が50→20に減らすことが出来ました。

と言ってもまだまだ、複雑度が高く、リファクタリングの余地があるのでこれからも続けていきたいと思っています。

また、コードデプロイ時に、循環的複雑度計測を自動化し、早期ボトルネックの発見するための仕組みづくりをしました。
今後は活用方法などブログで紹介していきます!

終わりに

現在スタディストでは、マニュアル作成・共有プラットフォームTeachme Bizを一緒に開発していくエンジニア、デザイナーの仲間を募集しています。

是非ともお気軽にお声がけ下さい。

--

--