🕌

Metrics/AbcSizeの目安を考えてみた

2023/08/12に公開

はじめに

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