redirect_toをもつコントローラメソッドに関するリファクタリングのベストプラクティス

Question

題名長くてすみません。
要は、下記のような汚いコードを想定し、これを直すときのベストプラクティスをご存知の方がいれば教えていただきたいです。

class examplesController 

  def methodEx(params)
   if params == A
    #処理1行
    #処理1行
　　 redirect_to A_path
   elsif params == B
    #処理1行
　　 #処理1行
　　 redirect_to B_path
   elsif params == c
    #処理1行
　　 #処理1行
    redirect_to C_path
   end
  end
end

というようなものです。メソッドで飛んできたパラメーターの値によってredirect先が変わります。
前提として、
1.paramsの種類が今後増えることはないです。
2.methodExはこのcontrollerのみでしか使用されません。
自分がこのコードの課題として感じているのは、途中の＃処理1行の部分もあるようにゴリゴリのロジックをcontrollerに書いていることです。これを直したいです。

リファクタリング案.1　処理１行の部分を全てモデルに移す
これは自分が一番最初に思いついたやつで実際に行いました。しかしそれでも、

class examplesController 

  def methodEx(params)
   if params == A
    #処理1行
　　 redirect_to A_path
   elsif params == B
　　 #処理1行
　　 redirect_to B_path
   elsif params == c
　　 #処理1行
    redirect_to C_path
   end
  end
end

このような感じになり、そこまで劇的によくなったかと言われると。。。という感じです。あまり行数も減ってない印象です。

リファクタリング案.2 モジュール化して場所を移す。
このmethodExをconcernsとかに移すこともできるかなと思いました。そうすればexamplesControllerも薄くなります。しかし、methodExが他のcontrollerで使われない特化したメソッドでであるとすると、あまりモジュールに移すのも意味がないように思えます。

この二つぐらいしか有用なもの（実際に有用かは不明）が思い浮かばず、もしより良い方法をご存知の方がいらっしゃいましたら、お教えいただきたいです。正直modelでredirect_toが動かせない以上、if分で切り分けて、それに応じて。。。という処理をcontrollerに書くのはしょうがないのではないかと思っています。ただfatなcontlloerにはなるなあとも思います。何卒よろしくお願いします。

Answer 1

まず、同じ変数で処理が分岐しているようですので case 文を適用することを考え付きました。

  def methodEx(params)
    case params
    when A
      #処理1行
　　  redirect_to A_path
    when B
　　  #処理1行
　　  redirect_to B_path
    when c
　　  #処理1行
      redirect_to C_path
    end
  end

次に、path を変数に入れる形にすることで、redirect_to の記載を一回にとどめます

  def methodEx(params)
    # この記法をやったことがないので、このままだと動かないかも
    destination_path = case params
      when A
        #処理1行
　　    A_path
      when B
　　    #処理1行
　　    B_path
      when c
　　    #処理1行
        C_path
      end
    redirect_to destination_path
  end

こうすると、redirect_to の行以外全てを、 model メソッドまたはcontrollerの privateメソッドへと移行可能です

※modelメソッドへ移行する場合の記載例になります

  def methodEx(params)
    destination_path = モデル名.hoge_method(params)
    redirect_to destination_path
  end

  # model 又は controller の praivate 以下
  def self.hoge_method(params)
    case params
    when A
      #処理1行
　　  A_path
    when B
　　  #処理1行
　　  B_path
    when c
　　  #処理1行
      C_path
    end
  end

3段階に分けてみましたが、たとえば3段階目だけど case文 ではなく if 文を使うというようなのもありだと思います。

Comments

[tomoharu]

確かにとてもスマートですね！ありがとうございます！