Flogを使用したリファクタリングの必要性の可視化と負債返却の説得の仕方の一例
複雑性の高い部分の探し方
コードの汚さを可視化したい
普段Rubyの開発をしていると、読みづらいと感じるコードに出会う時があります。自由にリファクタリングできたらいいのですが、仕事でのソフトウェア開発は特定期間の利益最大化を目的していることが多いので、プロダクト開発と負債返却の投資割合はビジネスインパクトと開発しにくさの比較での相談になることが多いです。
後数ヶ月で終わるプロダクトなら読みづらくてもこのまま行くという判断もあると思いますし、今後力を入れていく予定のところなら人をたくさん投入する前に綺麗なコードにしておくことで、開発効率x開発期間の面積の最大化ができるのでリファクタリングした方が投資としては成功になることもあるかと思います。
一方でコードが汚いからリファクタリングしたい、リファクタリングして綺麗になったというコードの良さは雰囲気で語られることも多く、一定の尺度があった方が議論がまとまりやすいように思いました。
Flogを使うとそういったトリアージが簡単に出来そうなので今回紹介します。
Flogの導入
Rubyのコードならなんでも動くのでSinatraでもHanamiでもいいのですが、一番マジョリティーのユースケースだと思うのでWeb開発でRuby on Railsを使っているケース前提で進めます。Redmineのコード(commit: e8db9db8948d928ab8e9dae75c261967676291a8)を使用。
Flogをインストールします。
# Gemfile
group :development do
gem "flog"
end
bundle install
flogをかけます。dir指定で多くの場合 app か lib になるかと
$ bundle exec flog app/
38137.0: flog total
15.5: flog/method average
456.0: ApplicationHelper#parse_redmine_links app/helpers/application_helper.rb:1088-1284
348.4: Query#sql_for_field app/models/query.rb:1218-1446
264.4: IssuesHelper#show_detail app/helpers/issues_helper.rb:524-658
251.6: Issue#none
189.4: User#none
185.1: Project#none
172.7: IssueImport#build_object app/models/issue_import.rb:113-241
167.9: Project#copy_issues app/models/project.rb:1122-1225
162.9: IssueQuery#initialize_available_filters app/models/issue_query.rb:147-280
160.8: Principal#none
158.8: IssuesController#bulk_edit app/controllers/issues_controller.rb:257-341
144.4: WorkflowTransition::replace_transitions app/models/workflow_transition.rb:23-91
141.3: Repository::Cvs#fetch_changesets app/models/repository/cvs.rb:128-200
136.9: ActivitiesController#index app/controllers/activities_controller.rb:25-85
136.0: Issue#validate_issue app/models/issue.rb:751-815
131.1: Issue#safe_attributes= app/models/issue.rb:555-633
128.0: IssuesHelper#render_descendants_tree app/helpers/issues_helper.rb:92-144
(後略)
今回の例で言うとapp配下のflog scoreが出力されます。
flog score、クラス名#メソッド名、ファイル名、行番号が出力されます。flog totalがscoreの総計、averageが平均。
内部的にflog scoreはAbc sizeを計測しているようです。デフォルトだとtop 60%のみの表示。
Flogの閾値変更、グルーピング
閾値はオプションで変更できます。
$ bundle exec flog -t 10 app/ # 上位10%
38137.0: flog total
15.5: flog/method average
456.0: ApplicationHelper#parse_redmine_links app/helpers/application_helper.rb:1088-1284
348.4: Query#sql_for_field app/models/query.rb:1218-1446
264.4: IssuesHelper#show_detail app/helpers/issues_helper.rb:524-658
251.6: Issue#none
189.4: User#none
185.1: Project#none
172.7: IssueImport#build_object app/models/issue_import.rb:113-241
167.9: Project#copy_issues app/models/project.rb:1122-1225
162.9: IssueQuery#initialize_available_filters app/models/issue_query.rb:147-280
160.8: Principal#none
158.8: IssuesController#bulk_edit app/controllers/issues_controller.rb:257-341
144.4: WorkflowTransition::replace_transitions app/models/workflow_transition.rb:23-91
141.3: Repository::Cvs#fetch_changesets app/models/repository/cvs.rb:128-200
136.9: ActivitiesController#index app/controllers/activities_controller.rb:25-85
136.0: Issue#validate_issue app/models/issue.rb:751-815
131.1: Issue#safe_attributes= app/models/issue.rb:555-633
128.0: IssuesHelper#render_descendants_tree app/helpers/issues_helper.rb:92-144
124.4: Query#statement app/models/query.rb:969-1026
123.9: AccountController#lost_password app/controllers/account_controller.rb:56-132
121.0: QueriesHelper#retrieve_query app/helpers/queries_helper.rb:345-395
118.9: IssuesController#bulk_update app/controllers/issues_controller.rb:343-424
クラス単位のグルーピング(兼ソート)も可能。
$ bundle exec flog -g app/
38137.0: flog total
15.5: flog/method average
1735.8: Issue total
251.6: Issue#none
136.0: Issue#validate_issue app/models/issue.rb:751-815
131.1: Issue#safe_attributes= app/models/issue.rb:555-633
112.3: Issue#after_create_from_copy app/models/issue.rb:1728-1797
102.4: Issue#recalculate_attributes_for app/models/issue.rb:1826-1877
61.9: Issue#project= app/models/issue.rb:413-452
61.6: Issue#workflow_rule_by_attribute app/models/issue.rb:685-732
60.5: Issue#validate_required_fields app/models/issue.rb:818-837
59.8: Issue#new_statuses_allowed_to app/models/issue.rb:1052-1093
52.7: Issue::visible_condition app/models/issue.rb:131-163
45.4: Issue#reschedule_on! app/models/issue.rb:1389-1423
43.1: Issue::update_versions app/models/issue.rb:1883-1899
42.6: Issue::load_visible_relations app/models/issue.rb:1227-1245
41.3: Issue#would_reschedule? app/models/issue.rb:1321-1338
41.2: Issue#css_classes app/models/issue.rb:1452-1466
37.5: Issue#after_project_change app/models/issue.rb:1697-1723
36.3: Issue::load_visible_last_notes app/models/issue.rb:1277-1294
35.9: Issue#notified_users app/models/issue.rb:1115-1129
34.7: Issue#copy_from app/models/issue.rb:295-323
34.6: Issue::cross_project_scope app/models/issue.rb:1535-1560
34.2: Issue#create_parent_issue_journal app/models/issue.rb:2007-2031
33.8: Issue#visible? app/models/issue.rb:166-188
33.0: Issue::load_visible_total_spent_hours app/models/issue.rb:1214-1224
32.6: Issue::load_visible_last_updated_by app/models/issue.rb:1258-1274
28.0: Issue#valid_parent_project? app/models/issue.rb:1517-1532
27.2: Issue::allowed_target_trackers app/models/issue.rb:1659-1676
26.1: Issue#visible_journals_with_index app/models/issue.rb:909-925
25.8: Issue#behind_schedule? app/models/issue.rb:974-979
25.6: Issue#assignable_versions app/models/issue.rb:999-1016
23.8: Issue::count_and_group_by app/models/issue.rb:1598-1617
23.3: Issue#tracker= app/models/issue.rb:379-393
1578.2: ApplicationHelper total
456.0: ApplicationHelper#parse_redmine_links app/helpers/application_helper.rb:1088-1284
113.6: ApplicationHelper#parse_wiki_links app/helpers/application_helper.rb:974-1038
85.3: ApplicationHelper#progress_bar app/helpers/application_helper.rb:1570-1602
76.8: ApplicationHelper#none
68.4: ApplicationHelper#format_object app/helpers/application_helper.rb:254-313
59.6: ApplicationHelper#render_page_hierarchy app/helpers/application_helper.rb:453-481
57.9: ApplicationHelper#page_header_title app/helpers/application_helper.rb:744-770
57.6: ApplicationHelper#render_project_nested_lists app/helpers/application_helper.rb:422-451
53.9: ApplicationHelper#textilizable app/helpers/application_helper.rb:848-893
53.0: ApplicationHelper#replace_toc app/helpers/application_helper.rb:1434-1465
43.7: ApplicationHelper#render_projects_for_jump_box app/helpers/application_helper.rb:533-568
42.9: ApplicationHelper#parse_non_pre_blocks app/helpers/application_helper.rb:895-925
42.8: ApplicationHelper#link_to_principal app/helpers/application_helper.rb:58-79
42.6: ApplicationHelper#project_tree_options_for_select app/helpers/application_helper.rb:595-616
41.1: ApplicationHelper#parse_inline_attachments app/helpers/application_helper.rb:936-962
40.8: ApplicationHelper#body_css_classes app/helpers/application_helper.rb:818-833
36.4: ApplicationHelper#render_project_jump_box app/helpers/application_helper.rb:571-593
35.8: ApplicationHelper#principals_options_for_select app/helpers/application_helper.rb:644-661
33.9: ApplicationHelper#link_to_issue app/helpers/application_helper.rb:104-122
32.0: ApplicationHelper#include_calendar_headers_tags app/helpers/application_helper.rb:1633-1659
28.3: ApplicationHelper#principals_check_box_tags app/helpers/application_helper.rb:625-641
27.2: ApplicationHelper#inject_macros app/helpers/application_helper.rb:1412-1429
25.4: ApplicationHelper#parse_headings app/helpers/application_helper.rb:1352-1373
23.1: ApplicationHelper#title app/helpers/application_helper.rb:773-783
1433.4: Query total
348.4: Query#sql_for_field app/models/query.rb:1218-1446
124.4: Query#statement app/models/query.rb:969-1026
104.3: Query#project_statement app/models/query.rb:935-967
(後略)
class名のみでまとめるオプションはないようなのでgrepを使います。(totalよりaverageが知りたい気もします)
$ bundle exec flog -g app/ |grep total |grep ':'
38137.0: flog total
1735.8: Issue total
33.0: Issue::load_visible_total_spent_hours app/models/issue.rb:1214-1224
1578.2: ApplicationHelper total
1433.4: Query total
27.8: Query#total_with_scope app/models/query.rb:1099-1112
1036.1: IssuesController total
917.1: IssueQuery total
909.9: IssuesHelper total
757.1: MailHandler total
732.5: Project total
657.5: User total
564.8: QueriesHelper total
24.9: QueriesHelper#available_totalable_columns_tags app/helpers/queries_helper.rb:113-128
442.4: RepositoriesController total
424.5: ApplicationController total
408.6: Mailer total
354.4: UsersController total
344.9: WikiController total
312.9: TimelogController total
311.4: RepositoriesHelper total
305.8: TimeEntry total
303.0: Attachment total
295.5: ProjectsController total
276.9: AccountController total
275.6: IssueImport total
254.2: Journal total
244.6: Repository::Cvs total
239.5: TimeEntryQuery total
232.0: Repository total
225.6: VersionsController total
205.2: Principal total
189.1: Version total
180.2: Setting total
174.1: WatchersController total
167.3: WorkflowsController total
166.8: Changeset total
156.7: WikiPage total
152.0: SearchHelper total
151.5: CustomField total
145.6: AuthSourceLdap total
144.4: WorkflowTransition total
143.0: Import total
(後略)
Redmineはプロジェクト管理ソフトなのでIssueやProject、メール周りのコードが厚くなるのはそんな気はしますね。データ分析はえてして当たり前の直感にあう結果が出ることも多いです。
何かしらの切り出しをした方が読みやすくなる可能性はあります。
Flog scoreと将来の変更ベースでのトリアージ
スコアが出たのでトリアージします。コードメトリクスは他にもCyclomatic ComplexityだったりPerceived Complexityだったりもあるので、これが体感と完全にあっているかは諸説あるんですが、いいところついているとは思います。
method単位で局所的に悪いところをなおしてもいいのですが、class単位、controllerとmodelを合わせたような概念単位で見た方がどういったドメインの取り回しが複雑になっているのかわかるので個人的にはよいように思います。
将来的な変更とflog scoreでDA表を作って判断します。
| 概念A | 概念B | 概念C | |
|---|---|---|---|
| flog score | 1500 | 800 | 50 |
| 開発予定 | ○ | ◎ | △ |
今までガンガン開発してきたからflog scoreが悪く今後も開発する可能性が高いという場合も普通にありそうですが、最初に手をつけるならscoreもそこそこ悪く、開発予定もある概念Bをとっかかりとしてリファクタリングしてみるのもいいのかなと。
チーム開発だと今アクティブに変更している部分に大きな修正を入れづらいと思うんですよね。コンフリクトしてしまうので。少し落ち着いていて、開発再開予定があり、コードが読みにくい部分を狙うのが成功法な気がします。
CIで事前に防ぐことはできるのか
その他の視点としてあまりやっている会社はないですが、CIでデフォルトbranchとの差分が出せると面白いかもしれないですね。
以前読んだMicrosoft Researchの研究によるとバグ予測のファクターとしてCode CoverageよりCode Complexityの方がRecallの高い指標でした。
自分の考えだとソフトウェアのバグというのは医療判断に近くPrecisionよりRecallの方が大事な認識です。ソフトウェアのバグという時、バグは稀な現象を指していると思って、全コード中で大丈夫な部分の割合が大きいとすると医療データに括りが近いのではないかと。
CIでテストが通っているかどうかを出している会社は多いものの、Abc Size他を出している会社は少ないと思うんですよね。あなたの作ったプルリクエストは既存ファイルの変更でいうと元値に対してコード複雑性スコア10上げていて、新規追加のファイルで言うとリポジトリ平均より20も高いです悔しいでしょう？ みたいなのを出すと綺麗なコードに対する意識レベルが上がるかなと。
Future Work
rubocopだとCyclomatic Complexity、Perceived Complexityを計測できるようなのでflog相当のことをやりたいです。
Discussion