Railsのコントローラのアクションがやたらと横に長くて読みにくいときに確認したいこと
こちらの記事*1の続きです。
長くなったので、アクションのコードがやたらと横に長いとき、について書きます。 残りは、パラメータの受け渡され方がよくわからないとき だー。
内容
- アクションのコードがやたらと横に長いとき
アクションのコードがやたらと横に長いとき
コントローラのコードが読みにくいときには、どれだけスクロールさせるんですか、と言いたくなるところまでいかないにしろ、 目の上下動だけではメソッドがよくわからないことになっていることもありますね*2。
そういう場合について、少し考えたことをメモしておきます。
条件分岐がひたすらネストしている
たまに、以下のようなひたすらネストしているようなコードに合う事があります。
if condition_1 if condition_2 some_command # condition_3の判定に必要 if conditoin_3 some_command # conditon_4の判定に必要 if condition_4 if conditon_5 @some_paramter = some_parameter else @some_paramter = some_parameter * other_parameter end end else raise error_a end end else raise error_b end
これも横に長くて困るコードではあるのですが、横に長くなくても読み難いのでやめてください。
if else end
は以外に対応する else
のブロックの行が何故そこを通過しているのか、ぱっと見わからなくなりがちです。
raise error_a
が何の条件のエラーか、折りたたみを使わずに常にすぐ分かる人はそれでいいのでしょうが、
そもそもそういう風に分岐を書く必要も無いです。
処理を進めない場合はすぐに返した方がいいと思います。
raise error_b unless condition_1 # 以下は conditoin_1 == true でなければ例外を発生させて行わない return unless condition_2 # conditon_2 == true でなければ処理を行わないので返していい some_command raise error_a unless conditoin_3 # 以下は conditoin_3 == true でなければ例外を発生させて行わない some_command return unless condition_4 # conditon_4 == true でなければ処理を行わないので返していい # 比較的単純な代入なら三項演算子を使った方がすっきりする @some_paramter = conditon_5 ? some_parameter : some_parameter * other_parameter
ネストしているリクエストパラメータやsessionの値の受け渡しを全部開いて書いてる
また、横に長くなるパターンとしてフォームの入れ子のようなことをしたりして、 パラメータやsessionのハッシュがネストしすぎている、という場合も考えられます。
def some_action @user_name = params[:register_form][:personal_info][:user][:name] @user_age = params[:register_form][:personal_info][:user][:age] @user_var1 = params[:register_form][:personal_info][:user][:var1] end
上の例は極端ですし、この程度ならまだ平気なのですが、これが、条件分岐のネストの中に入ってきて、 sessionのネストとリクエストパラメータのネストが並ぶと辛いものがあります。
def some_action if condition_1 session[:temporary][:user][:name] = params[:register_form][:personal_info][:user][:name] session[:temporary][:user][:age] = params[:register_form][:personal_info][:user][:age] session[:temporary][:user][:var1] = params[:register_form][:personal_info][:user][:var1] end end
例は適当なんでこういう例は実際にないんですが。
あまりにも長いパラメータが複数のアクションで頻出する場合は、そのパラメータのネストしている部分をメソッドで呼び出せるようにしておくと読みやすくなるし、タイポも減るかも。
def some_action if condition_1 temp_user_session[:name] = register_user_params[:name] temp_user_session[:age] = register_user_params[:age] temp_user_session[:var1] = register_user_params[:var1] end end private def temp_user_session session[:temporary][:user] end def register_user_params params[:register_form][:personal_info][:user] end
こういうパラメータに別名を振るような真似、コードがあっちこっち散るような気がしましたが、適切に名前をつけておけば、そのコントローラ見るとき一回読んだら覚えられる感じだったので、ネストの間違いとかタイポを減らして*3、目線を上下移動だけに集中させられるメリットが勝りそう。
1つのアクションの1カ所にしか出て来ないんだったら、その行が連続して出てくる部分の上でやればよくて、メソッドにするのはやりすぎです。
def some_action temp_user_session = session[:temporary][:user] register_user_params = params[:register_form][:personal_info][:user] if condition_1 temp_user_session[:name] = register_user_params[:name] temp_user_session[:age] = register_user_params[:age] temp_user_session[:var1] = register_user_params[:var1] end end
また、こういった長めのパラメータの呼び出しが複数のコントローラで使用されるようであれば、 ヘルパなり、それらのコントローラの基底コントローラなりを用意して、そこにメソッドを置いておくとよいのかもしれないです。
*2:それに見合う横長のデュアルディスプレイがあれば平気とかそういう話は(ry
*3:ネストの間違いやタイポの間違いは、 undefined method [] for Nil Class の形で出てくるので少し面倒くさい