ダムを構築し、スマートにリファクタリングする：RubyonRailsコードから問題をマッサージする方法

時々、クライアントは私たちが本当に嫌いな機能要求を私たちに与えます。 私たちがクライアントを好きではないということではありません。私たちはクライアントを愛しています。 この機能が気に入らないわけではありません。クライアントから要求された機能のほとんどは、ビジネスの目標と収入に完全に一致しています。

機能リクエストを解決する最も簡単な方法は悪いコードを書くことであり、頭のてっぺんに洗練されたソリューションがないため、機能リクエストが気に入らない場合があります。 これにより、Rails開発者の多くは、RubyToolbox、GitHub、開発者ブログ、StackOverflowを介して無駄な検索を行い、自分自身について気分を良くする宝石やプラグイン、またはサンプルコードを探します。

RubyonRailsコードのリファクタリング

さて、私はあなたに言うためにここにいます：悪いコードを書いても大丈夫です。 時々、悪いRailsコードは、時間の制約の下で実装されたよく考えられていないソリューションよりも、美しいコードにリファクタリングする方が簡単です。

これは、私の恐ろしいバンドエイドソリューションから問題をマッサージするときに私が従うのが好きなRailsリファクタリングプロセスです。

別の観点として、段階的にリファクタリングされた機能のGitコミットログを次に示します。

そして、Toptalネットワークの同僚からの大規模なリファクタリングに関する別の興味深い記事があります。

それがどのように行われるか見てみましょう。

ビュー

ステップ1.ビューから開始します

新機能のチケットを開始するとします。 クライアントは、「訪問者はウェルカムページでアクティブなプロジェクトのリストを表示できるはずです」と言っています。

このチケットには目に見える変更が必要なので、作業を開始するのに適切な場所はビューです。 問題は単純明快であり、私たち全員が何度も解決するように訓練されています。 私はそれを間違った方法で解決し、ソリューションを適切な領域にリファクタリングする方法を示します。 問題の解決間違った方法は、正しい解決策がわからないというこぶを乗り越えるのに役立ちます。

まず、 activeというブール属性を持つProjectというモデルがあると仮定します。 activeがtrueに等しいすべてのProjectsのリストを取得したいので、 Project.where(active: true)を使用して、 eachブロックでループすることができます。

 app/views/pages/welcome.haml: %ul.projects - Project.where(active: true).each do |project| %li.project= link_to project_path(project), project.name

私はあなたが言っていることを知っています：「それはコードレビューに合格することは決してないだろう」または「私のクライアントは確かにこれのために私を解雇するだろう」。 はい、このソリューションは、Model-View-Controllerの関心の分離を破り、追跡が困難なデータベース呼び出しを迷わせる可能性があり、将来的に維持することが困難になる可能性があります。 しかし、それを行うことの価値を考慮してください間違った方法：

1. この変更は、15分以内にステージングで取得できます。
2. そのままにしておくと、このブロックは簡単にキャッシュできます。
3. このRailsの問題の修正は簡単です（後輩の開発者に渡すことができます）。

ステップ2.パーシャル

間違った方法でそれを行った後、私は自分自身について気分が悪くなり、自分の悪いコードを切り分けたいと思っています。 この変更が明らかにビューレイヤーの懸念事項である場合は、恥を部分的にリファクタリングすることができます。

 app/views/pages/welcome.haml: = render :partial => 'shared/projects_list' app/views/shared/projects_list.haml: %ul.projects - Project.where(active: true).each do |project| %li.project= link_to project_path(project), project.name

それは少し良いです。 明らかに、ビューでのモデルクエリの間違いをまだ犯していますが、少なくともメンテナが後でやって来て私の恐ろしい部分を見ると、特にRailsコードの問題に簡単に取り組むことができます。 明らかに馬鹿げたものを修正することは、実装が不十分でバグのある抽象化を修正するよりも常に簡単です。

ステップ3.ヘルパー

Railsのヘルパーは、ビューのセクションにDSL（ドメイン固有言語）を作成する方法です。 hamlやHTMLの代わりにcontent_tagを使用してコードを書き直す必要がありますが、他の開発者が15行の非印刷ビューコードを睨むことなくデータ構造を操作できるという利点があります。

ここでヘルパーを使用する場合は、おそらくliタグをリファクタリングします。

 app/views/shared/projects_list.haml: %ul.projects - Project.where(active: true).each do |project| = project_list_item(project) app/helpers/projects_helper.rb: def project_list_item(project) content_tag(:li, :class => 'project') do link_to project_path(project), project.name end end

コントローラー

ステップ4.コントローラーに移動します

ひどいコードは、ビューの問題だけではないかもしれません。 それでもコードの匂いがする場合は、ビューからコントローラーに移行できるクエリを探してください。

 app/views/shared/projects_list.haml: %ul.projects - @projects_list.each do |project| = project_list_item(project) app/controllers/pages_controller.rb: def welcome @projects = Project.where(active: true) end

ステップ5.コントローラーフィルター

コードをControllerbefore_filterまたはbefore_filterに移動する最も明白な理由は、複数のControllerアクションで複製したコードのためafter_filter 。 コントローラーアクションの目的をビューの要件から分離する場合は、コードをコントローラーフィルターに移動することもできます。

 app/controllers/pages_controller.rb: before_filter :projects_list def welcome end def projects_list @projects = Project.where(active:true) end

ステップ6.アプリケーションコントローラー

すべてのページにコードを表示する必要がある場合、またはすべてのコントローラーでコントローラーヘルパー関数を使用できるようにする場合は、関数をApplicationControllerに移動できます。 変更がグローバルである場合は、アプリケーションのレイアウトも変更することをお勧めします。

 app/controllers/pages_controller.rb: def welcome end app/views/layouts/application.haml: %ul.projects - projects_list.each do |project| = project_list_item(project) app/controllers/application_controller.rb: before_filter :projects_list def projects_list @projects = Project.where(active: true) end

モデル

ステップ7.モデルへのリファクタリング

MVCのモットーとして、 Fat Model、Skinny Controllers、およびDumbViewsがあります。 可能な限りすべてをモデルにリファクタリングすることが期待されており、最も複雑な機能が最終的にモデルの関連付けとモデルメソッドになることは事実です。 モデルでフォーマット/表示を行うことは常に避ける必要がありますが、データを他のタイプのデータに変換することは許可されています。 この場合、モデルにリファクタリングするのに最適なのはwhere(active: true)句で、これをスコープに変えることができます。 スコープを使用すると、呼び出しが見栄えが良くなるだけでなく、 deleteやoutdatedなどの属性を追加することにした場合は、 where句をすべて探す代わりに、このスコープを変更できます。

 app/controllers/application_controller.rb: before_filter :projects_list def projects_list @projects = Project.active end app/models/project.rb: scope :active, where(active: true)

ステップ8.モデルフィルター

この場合、モデルのbefore_saveまたはafter_save filtersを特に使用する必要はありませんが、私が通常行う次のステップは、機能をコントローラーとモデルメソッドからモデルフィルターに移動することです。

別の属性num_viewsがあるとします。 num_views > 50の場合、プロジェクトは非アクティブになります。 ビューでこの問題を解決することはできますが、ビューでデータベースを変更することは不適切です。 コントローラーで解決できますが、コントローラーはできるだけ薄くする必要があります。 モデルで簡単に解くことができます。

 app/models/project.rb: before_save :deactivate_if_over_num_views def deactivate_if_over_num_views if num_views > 50 self.active = false fi end

注：モデルフィルターでself.saveを呼び出さないようにする必要があります。これにより、再帰的な保存イベントが発生し、アプリケーションのデータベース操作レイヤーはとにかくコントローラーである必要があります。

ステップ9.ライブラリ

場合によっては、機能が十分に大きいため、独自のライブラリが必要になることがあります。 多くの場所で再利用されているため、または個別に開発したいほど大きいため、ライブラリファイルに移動することをお勧めします。

ライブラリファイルをlib/ディレクトリに保存するのは問題ありませんが、ライブラリファイルが大きくなるにつれて、実際のRubyGemに転送できます。 コードをライブラリに移動する主な利点は、モデルとは別にライブラリをテストできることです。

とにかく、プロジェクトリストの場合、 scope :active呼び出しをProjectモデルからライブラリファイルに移動し、それをRubyに戻すことを正当化できます。

 app/models/project.rb: class Project < ActiveRecord::Base include Activeable before_filter :deactivate_if_over_num_views end lib/activeable.rb: module Activeable def self.included(k) k.scope :active, k.where(active: true) end def deactivate_if_over_num_views if num_views > 50 self.active = false end end end

注： self.includedメソッドは、Rails Modelクラスがロードされ、クラススコープに変数kとして渡されるときに呼び出されます。

結論

このRubyonRailsリファクタリングのチュートリアルでは、15分以内に解決策を実装し、ユーザーテストのためにステージングして、機能セットに受け入れるか削除する準備をしました。 リファクタリングプロセスの終わりまでに、最も厳しいレビュープロセスでさえも合格する、複数のモデルにわたってリスト可能でアクティブ化可能なアイテムを実装するためのフレームワークをレイアウトするコードができました。

独自のRailsリファクタリングプロセスで、自信がある場合は、パイプラインの数ステップをスキップしてください（たとえば、ビューからコントローラー、またはコントローラーからモデルにジャンプします）。 コードの流れはビューからモデルへであることに注意してください。

愚かに見えることを恐れないでください。 現代語と古いCGIテンプレートレンダリングアプリケーションを区別するのは、毎回すべてを正しい方法で行うことではありません。リファクタリング、再利用、および取り組みの共有に時間をかけることです。