ruby-on-rails - DRY-ing up 開始 - レスキュー - 終了

Question

私はこの作成方法を持っています：

def create
    ...
    grid = Tag.find_by_id(story[:tag_id]) or raise GridNotFoundError
    ...
    event = Event.find_by_id(story[:event_id]) or raise EventNotFoundError
    ...
  rescue GridNotFoundError
    flash.now[:error] = "Please select a category"
    @item = item
    @years = story[:years]
    @event_name = race[:name]
    @country = race[:country]
    @classification = race[:class]
    @events = Event.all
    @countries = Tag.countries
    @classifications = Classification.all
    @grids = Tag.grids.find(:all, :conditions => ["value != ?", "Channel Creation Grid"])
    render "home/race_updates"
  rescue EventNotFoundError
    flash.now[:error] = "Please select an event or create a new one if you don't find your event"
    @item = item
    @event = story[:event_id]
    @years = story[:years]
    @events = Event.all
    @countries = Tag.countries
    @classifications = Classification.all
    @grids = Tag.grids.find(:all, :conditions => ["value != ?", "Channel Creation Grid"])
    render "home/race_updates"
  rescue CreateEventError
    flash.now[:error] = "There has been a problem creating your event."
    params[:expand] = true
    @item = item
    @years = story[:years]
    @event_name = race[:name]
    @country = race[:country]
    @classification = race[:class]
    @events = Event.all
    @countries = Tag.countries
    @classifications = Classification.all
    @grids = Tag.grids.find(:all, :conditions => ["value != ?", "Channel Creation Grid"])
    render "home/race_updates"
  rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound
    flash.now[:error] = item.errors.full_messages.join(" ,")
    @item = item
    @event = story[:event_id]
    @years = story[:years]
    @events = Event.all
    @countries = Tag.countries
    @classifications = Classification.all
    @grids = Tag.grids.find(:all, :conditions => ["value != ?", "Channel Creation Grid"])
    render "home/race_updates"
  end

ご覧のとおり、レスキューはほとんど同じです。レスキューは、home#race_updates メソッドの文字通りのコピー アンド ペーストでもあります。

2 つの質問があります。

1. これを乾かす方法はありますか？
2. これは一般的にコントローラーにとって良いパターンですか?

関数として分離することを考えましたが、フラッシュ メッセージ、アイテム、ストーリー、レース変数を渡す必要があります。エレガントなソリューションではないように感じますが、確かにクリーンになります。

このようにコーディングする (つまり、エラーを発生させて解決する) と、実際のビジネス ロジックを本来あるべき姿に分離し、ビジネス ロジックで発生するさまざまなエラー/ケースを処理することが容易になることがわかりました。これまでのところ動作していますが、これがベスト プラクティスなのか、意図したとおりに begin/rescue を使用していないのかについて意見を集めたいですか?

Answer 1

これを DRY することは素晴らしいアイデアであり、可能であれば Rails 設計 (およびテスト駆動開発/TDD) の良い教訓になります。

理想的には、次のようにします。

def create
    ...
    grid = Tag.find_by_id(story[:tag_id]) or raise GridNotFoundError
    ...
    event = Event.find_by_id(story[:event_id]) or raise EventNotFoundError
    ...
  rescue GridNotFoundError
    flash.now[:error] = "Please select a category"
    process_grid_not_found(item, story, race, etc...)
  rescue EventNotFoundError
    flash.now[:error] = "Please select an event or create a new one if you don't find your event"
    process_event_not_found(item, story, race, etc...)
  rescue CreateEventError
    flash.now[:error] = "There has been a problem creating your event."
    process_event_create_error(item, story, race, etc...)
  rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound
    flash.now[:error] = item.errors.full_messages.join(" ,")
    process_other_error(item, story, race, etc...)
  end
  render 'home/race_updates'

次に、関連する新しいメソッド (process_event_not_foundなど) をprivateモデル内の別の (おそらく) メソッドとして作成する必要があります。

これにより、コードがはるかに読みやすくなりますが、テスト コードをより簡単に記述できるという大きな利点があります。

したがって、個々の例外メソッドのそれぞれに必要な分離された機能をテストするテスト コードを (Test::Unitまたはその他を使用して) 記述する必要があります。rspecこれにより、より優れたコードが得られるだけでなく、例外メソッドがより小さくモジュール化されたメソッド自体に分解される可能性が高いことがわかります。

Ruby と Rails の開発者がテスト駆動開発の利点について話しているのを聞くとき、そのアプローチの主な利点の 1 つは、ここで説明したような長くて複雑なメソッドになる可能性がはるかに低いことです。より小さく、より単純なメソッドを使用して、より DRY なコードを作成する可能性が高くなります。

また、これを理解したら、もう一度見て、さらに単純化することをお勧めします。単純化の余地はもっとありますが、繰り返しリファクタリングを行い、説明したように分解し、テストを開始することから始めることをお勧めします。

Answer 2

ケビン・ベデルの答えとビクター・モロースの洞察を組み合わせ、Ruby ではandとorがフロー制御構造であるという事実を組み合わせました。私はこれを思いついた：

def create
    ...
    grid = Tag.find_by_id(story[:tag_id]) or (render_race_updates(item, "Please select a category") and return)
    ...
    event = Event.find_by_id(story[:event_id]) or (render_race_updates(item, "Please select an event or create a new one if you don't find your event") and return)
    ...
    if item.save
      ...
    else
      render_race_updates item, item.errors.full_messages.join(", ")
    end
  rescue CreateEventError
    params[:expand] = true
    render_race_updates item, "There has been a problem creating your event."
  end
private
  def render_race_updates(item, message)
    flash.now[:error] = message
    # etc.
    render "home/race_updates"
  end

このようにして、他のメソッドによってバブルアップされた例外をキャッチしながら、例外を発生させずに発生する例外的なケースを処理することができます。

ただし、まだテストを作成する必要があります。今のところ、単体テストのみを書いています。まだ RSpec のコツをつかんでおり、ゆっくりと私の "wing it" 開発を TDD に変更していますが、それはまったく別の会話です。

Answer 3

これがまだ 100% 正しいかどうかは、自分で確認する必要があります。

def create
    ...
    grid = Tag.find_by_id(story[:tag_id]) or raise GridNotFoundError
    ...
    event = Event.find_by_id(story[:event_id]) or raise EventNotFoundError
    ...
rescue Exception => e
    flash.now[:error] = e.is_a?(GridNotFoundError) ? "Please select a category" :
                        e.is_a?(EventNotFoundError) ? "Please select an event or create a new one if you don't find your event" : 
                        e.is_a?(CreateEventError) ? "There has been a problem creating your event." :
                        e.is_a?(ActiveRecord::RecordNotSaved) or e.is_a?(ActiveRecord::RecordInvalid) or e.is_a?(ActiveRecord::RecordNotFound) ? item.errors.full_mesages.join(", ") : e.to_s
    @item = item
    @years = story[:years]
    @event_name = race[:name] unless e.is_a?(EventNotFoundError) or e.is_a?(ActiveRecord::RecordNotSaved) or e.is_a?(ActiveRecord::RecordInvalid) or e.is_a?(ActiveRecord::RecordNotFound)
    @events = Event.all
    @countries = Tag.countries
    @classifications = Classification.all
    @grids = Tag.grids.find(:all, :conditions => ["value != ?", "Channel Creation Grid"])
    @event = story[:event_id] if e.is_a?(EventNotFoundError) or e.is_a?(ActiveRecord::RecordNotSaved) or e.is_a?(ActiveRecord::RecordInvalid) e.is_a?(ActiveRecord::RecordNotFound)

    if e.is_a?(GridNotFoundError) or e.is_a?(CreateEventError) 
        @country = race[:country]
        @classification = race[:class]

    end

    params[:expand] = true if e.is_a?(CreateEventError)

    render "home/race_updates"
end