🕌
Metrics/AbcSizeの目安を考えてみた
はじめに
RuboCopとは
RuboCopは、Rubyの静的コード解析ツールであり、コードの品質やスタイルを向上させるために広く使用されています。コードベース内の潜在的な問題やスタイルガイドに違反する箇所を特定し、開発者がより一貫したコードを書くのに役立ちます。
Metrics/AbcSizeについて調べようと思ったきっかけ
関わっているプロジェクトで# rubocop:disable
でRuboCopを無効化している箇所が多く、どのぐらいあるか調べてみたところ以下の結果になりました。
特に一番使用回数が多いMetrics/AbcSizeに焦点を当てて「適切な設定値なのか」を考えてることにしました。
RuboCop Rule | Count | Description |
---|---|---|
Metrics/AbcSize | 56 | 測定するメソッドの複雑さ。 |
Rails/SkipsModelValidations | 21 | モデルのバリデーションをスキップするメソッドの使用。 |
Metrics/ClassLength | 15 | クラスの長さを制限。 |
Metrics/CyclomaticComplexity | 12 | サイクロマティック複雑度を測定。 |
Metrics/BlockLength | 6 | ブロックの長さを制限。 |
Rails/UniqueValidationWithoutIndex | 4 | ユニークバリデーションがインデックスなしで使用されている。 |
Rails/LexicallyScopedActionFilter | 5 | アクションフィルターのスコープ問題を検出。 |
Metrics/PerceivedComplexity | 4 | 視認複雑度を測定。 |
Layout/LineLength | 2 | 1行の最大長を制限。 |
Metrics/ParameterLists | 2 | メソッドのパラメータ数を制限。 |
Style/RedundantSelf | 1 | 不要なselfの使用を検出。 |
Naming/HeredocDelimiterNaming | 1 | HEREドキュメントのデリミタの命名規約。 |
Rails/I18nLocaleTexts | 1 | 固定のロケールテキストの使用を検出。 |
Rails/HasManyOrHasOneDependent | 1 | :dependent オプションがない has_many/has_one 関連。 |
Rails/InverseOf | 1 | :inverse_of オプションがないActiveRecordの関連。 |
Style/MultilineBlockChain | 1 | 複数行にわたるブロックチェーンのスタイル。 |
Rails/CreateTableWithTimestamps | 1 | create_table でのタイムスタンプの使用。 |
Rails/ReversibleMigration | 1 | reversibleなマイグレーションの推奨。 |
all | 1 | すべてのCopを無効にするディレクティブ。 |
Total | 139 | ー |
Metrics/AbcSizeとは?
Metrics/AbcSizeは、コードの複雑性を評価するための一つのメトリクスです。
Aはメソッド内の代入文(Assignments)、B は分岐(Branches)、C は条件(Conditions)を表し、三つをそれぞれ二条の平方根がAbcSizeとなります。
この値が大きいほど、メソッドの複雑性が高いとされます。
各プロジェクトの調査(設定値について考察)
設定値の目安にするため、いくつかのプロジェクトMetrics/AbcSizeを調べました。
リポジトリ | Metrics/AbcSize(MAX設定) |
---|---|
Rubocop | 24 |
GitLab | 30 |
rubygems.org | 42 |
spree | 45 |
数値だけだとイメージがつかないと思うので、数値と実装コード例を記載します。
- Metrics/AbcSize 27の場合
def execute(start_id:)
current_start_id = start_id
return build_result(next_start_id: nil) if max_id.nil?
return build_result(next_start_id: min_id) if current_start_id > max_id
@start_time = monotonic_time
result[:mismatches] = result[:mismatches_details].length
metrics_counter.increment({ source_table: source_model.table_name, result: 'match' },result[:matches])
metrics_counter.increment({ source_table: source_model.table_name, result: 'mismatch' },result[:mismatches])
build_result(next_start_id: current_start_id > max_id ? min_id : current_start_id)
end
- Metrics/AbcSize 41の場合
def execute(start_id:)
current_start_id = start_id
return build_result(next_start_id: nil) if max_id.nil?
return build_result(next_start_id: min_id) if current_start_id > max_id
@start_time = monotonic_time
MAX_BATCHES.times do
if (current_start_id <= max_id) && !over_time_limit?
ids_range = current_start_id...(current_start_id + BATCH_SIZE)
current_start_id += BATCH_SIZE
result[:matches] += append_mismatches_details(source_data, target_data)
result[:batches] += 1
else
break
end
end
result[:mismatches] = result[:mismatches_details].length
metrics_counter.increment({ source_table: source_model.table_name, result: 'match' },result[:matches])
metrics_counter.increment({ source_table: source_model.table_name, result: 'mismatch' },result[:mismatches])
build_result(next_start_id: current_start_id > max_id ? min_id : current_start_id)
end
- Metrics/AbcSize 52の場合
def execute(start_id:)
current_start_id = start_id
return build_result(next_start_id: nil) if max_id.nil?
return build_result(next_start_id: min_id) if current_start_id > max_id
@start_time = monotonic_time
MAX_BATCHES.times do
if (current_start_id <= max_id) && !over_time_limit?
ids_range = current_start_id...(current_start_id + BATCH_SIZE)
source_data = source_model.where(source_sort_column => ids_range).
order(source_sort_column => :asc).pluck(*source_columns)
target_data = target_model.where(target_sort_column => ids_range).
order(target_sort_column => :asc).pluck(*target_columns)
current_start_id += BATCH_SIZE
result[:matches] += append_mismatches_details(source_data, target_data)
result[:batches] += 1
else
break
end
end
result[:mismatches] = result[:mismatches_details].length
metrics_counter.increment({ source_table: source_model.table_name, result: 'match' },result[:matches])
metrics_counter.increment({ source_table: source_model.table_name, result: 'mismatch' },result[:mismatches])
build_result(next_start_id: current_start_id > max_id ? min_id : current_start_id)
end
27の場合は注意深く読む必要があるが許容範囲内で、41、52の場合はかなり読みにくさを感じました。。
まとめ
30までは許容範囲内で40を超えてくるとかなり読みにくさを個人的に感じました。
数値が小さいに越したことはありませんが、小さすぎると制限が厳しすぎるので以下を目安にして決めるのもいいかと思いました。
- 15-20: ほとんどの人にとって理解しやすい
- 20-30: 注意深く読む必要がある場合がある
- 30-40: 非常に複雑で、リファクタリングを検討するべき
- 45以上: 可能な限り分割または再構築するべき
Discussion