リファクタリングの判断基準にメソッドの呼び出し階層の深さはあるか知りたい

Question

前提・実現したいこと

元々Java開発をしていましたが、現在Rails開発に携わっています。

メソッド階層が多いと個人的には読みづらいと思うのですが、リファクタリングの判断基準にそのような視点はありますでしょうか？

例えば以下のコードがあるとき

  def delete_foo(x, y)
    (1..5).each do |i|
        {
          "hoge_id_#{i}": x,
          "hoge_value_#{i}": y,
        }
      end
    end
  end

  def update_foo(x, y, z)
    (1..5).each do |i|
      if z == hoge.public_send("hoge_#{i}_id")
        {
          "hoge_id_#{i}": x,
          "hoge_value_#{i}": y,
        }
      end
    end
  end

  def create_foo(x, y, z)
    (1..5).each do |i|
      unless z.include?(i)
        {
          "hoge_id_#{i}": x,
          "hoge_value_#{i}": y,
        }
      end
    end
  end

この部分をまとめられるので

 {
      "hoge_id_#{i}": x,
      "hoge_value_#{i}": y,
 }

以下のように修正すると、メソッドの呼び出し階層が深くなり、読みづらい印象を受けました。(まだ下のコードはシンプルなのでそこまで感じないかもですが、)

  def delete_foo(x, y)
    (1..5).each do |i|
        params
      end
    end
  end

  def update_foo(x, y, z)
    (1..5).each do |i|
      if z == hoge.public_send("hoge_#{i}_id")
        params
      end
    end
  end

  def create_foo(x, y, z)
    (1..5).each do |i|
      unless z.include?(i)
        params
      end
    end
  end

  def params
    {
        "hoge_id_#{i}": x,
        "hoge_value_#{i}": y,
     }
  end

試したこと

・「リファクタリング　メソッド 呼び出し階層 深さ」などで調べる

Comments

[Zuishin]

読みにくくなったようには見えませんが、なぜそう思ったのでしょうか？
逆に params という定義が一か所に収まったおかげで、三か所すべてで間違い探しをせずに済んで読みやすくなりました。
(引数を正しく使い、まともに動くものができたと仮定しての話です)

強いて言うなら、params という名前が問題かもしれません。
hoge_ なんとかをまとめた無意味なデータなので、この例ではまともな命名はできませんが、これがちゃんとしたデータであれば、person や commodity など読みやすい命名ができると思います。

[k499778]

ご返答ありがとうございます！
たしかに上記コードですと、メソッド化した方がむしろ読みやすいと思うのもわかる気がします。

実際に自分が修正しているコードは、もう少し複雑で、delete_fooなどを呼び出す処理自体も複雑だったりして、
呼び出したメソッドから、さらに呼び出されていると直感的にわかりづらい印象を受けました。

たしかに客観的に考えるとメソッド化した方が良さそうな気はしたのですが、主観は微妙かなと思い、決め手となる判断基準を調べておりました。

[k499778]

たしかにメソッド名は大事ですよね

Answer 1

リファクタリングの判断基準にそのような視点はありますでしょうか？

新装版 リファクタリングの第３章「コードの不吉な臭い」で「リファクタリングの必要性を示す不吉な兆候について説明」されています。

重複したコード、長すぎるメソッドなどが挙げられていますが、呼び出し階層の深さはリファクタリングの判断基準とはされていません。

私としては、開発者がリファクタリングした方がよいと感じるなら、すればいいと思います。

本章で、リファクタリングがいつ要求されるかについての正確な基準を設けるつもりはありません。経験で磨かれた人間の直感には、メトリックスをいくら集めてもかなわないものです。

Comments

[k499778]

ありがとうございます！判断基準には含まれていないことを知れて良かったです。