Rubocop のコーディング規約をもとに読みやすいコードについて考えた

こんにちは。ソリューション技術部OTTサービスソリューション統括部LOGICAプロダクトグループの田上です。

最近、チームリーダー的な立場でソースコードレビューすることも多く、また不具合発生などにより過去の処理を読み解くことがよくあります。

その際、複雑でわかりにくいコードなどがあると処理を追うのに時間がかかり、また複雑なコードほどバグが発生する確率が高いです。

現在、私が使用しているRubyにはRubocopというコーディング規約を設定してチェックするツールがあります。

github.com

Rubocopを導入するとコーディング規約で制限するので、全体的に書き方が統一され第三者が見たとき読みやすいコーディングとなります。

そこで、Rubocopでよく発生するエラーを例に、第三者の観点からどのようなコーディングが読みづらいか取り上げてみたいと思います。

Rubocopのエラー

Metrics/MethodLength：メソッドのステップ数が多い

メソッドのステップ数が多くなると第三者が見て理解するのが難しくなります。

また、エディタ内にメソッドが全て表示されないと、画面を縦スクロールする必要がありレビューワーがストレスを感じます。

解消法としては、メソッド内の処理を別メソッドに切り出す、三項演算子・後置ifを使用するなどがあります。

# 変更前
def message(value)
  if value.nil?
    return 'no value'
  end

  if value =~ /^[0-9]+$/
    return "number value:#{value}"
  end

  len = 100
  if value.length > len
    return "#{value.slice(0, len - 3)}..."
  end

  value
end

# 変更後
def message(value)
  return 'no value' if value.nil?

  return "number value:#{value}" if value =~ /^[0-9]+$/

  truncate(value, 100)
end

def truncate(value, len)
  value.length > len ? "#{value.slice(0, len - 3)}..." : value
end

私がメソッドを切り分ける基準としては、メソッドの処理をコメント1行で書けるほどにシンプルにする、という感じで教わりました。

なお、Rubocopのデフォルトの閾値は10ですが、今までの設定実績では20ほどで運用していました。

Layout/LineLength：1行が長い

1行が横に長いとエディタ上で途中で折り返され、内容を理解し難くなります。

解消法としては、適度な改行を入れる、長い変数名の見直しなどがあります。

なお、Rubocopのデフォルトの閾値は120でデフォルト値で運用しておりました。

Style/GuardClause：Guard Clause（ガード節）の使用を推奨

メソッドの冒頭部分で条件判定して処理を終了し、メインの処理へ影響を与えないようにします。

# 変更前
def names(books)
  if books.empty?
    return []
  else
    return books.map(&:name)
  end
end

# 変更後
def names(books)
  return [] if books.empty?

  books.map(&:name)
end

レビューワーからすると、冒頭部分での処理を実施しない条件を見分けやすく、メインの処理で何をやっているか判断しやすくなります。

Layout/EmptyLineAfterGuardClause：Guard Clause 後に空行が無い

上記で説明したGuard Clause 後に空行が無いパターン。

# 変更前
def names(books)
  return [] if books.nil?
  books.map(&:name)
end

# 変更後
def names(books)
  return [] if books.nil?

  books.map(&:name)
end

Guard Clause 後に空行が無いと、視覚的に処理が続いていると錯覚しそうになります。

Metrics/CyclomaticComplexity：メソッドが複雑

CyclomaticComplexity（循環的複雑度）は、メソッド内の条件判定（if/unless/when/and/or）や繰り返し処理（while/until/for）のカウント数となります。

def numeral(num)
  return 'zero' unless num  #1

  case num
  when 1 then 'first'       #2
  when 2 then 'second'      #3
  when 3 then 'third'       #4
  when 4 then 'fourth'      #5
  when 5 then 'fifth'       #6
  when 6 then 'sixth'       #7
  when 7 then 'seventh'     #8
  when 8 then 'eighth'      #9
  when 9 then 'ninth'       #10
  when 10 then 'tenth'      #11
  else 'more'               #12
  end
end

上記の例では複雑度のカウント数は12となります。

解消法としては、メソッドの処理を別処理への切り出し、そもそものメソッドの処理の見直しが必要です。

NUMERAL_LIST = {
  1 => 'first', 2 => 'second', 3 => 'third',
  4 => 'fourth', 5 => 'fifth', 6 => 'sixth',
  7 => 'seventh', 8 => 'eighth', 9 => 'ninth',
  10 => 'tenth'
}.freeze

def numeral(num)
  return 'zero' unless num

  str = NUMERAL_LIST[num]
  return 'more' if str.nil?

  str
end

また、Metrics/MethodLengthなど他のエラーなどを解消することにより、同時に解消される場合が多いと思います。

なお、Rubocopのデフォルトの閾値は7ですが、こちらはデフォルト値で運用しておりました。

最後に

今回はRubocopのエラーを例に読みづらいコーディングを取り上げました。

Rubocopには自動補正機能などもあり、また単純な文法間違いなどは処理を実行する前に早期に発見できます。

新たにプロジェクトを立ち上げる場合や、既存プロジェクトのレビューなどで困っている場合は導入してはいかがでしょうか？