干燥开始救援结束

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. 这对于一般的控制器来说是一个好的模式吗？

我曾考虑将它作为一个函数分离出来，但我需要传入 Flash 消息、项目、故事和比赛变量。我觉得这不是一个优雅的解决方案，但它肯定会更干净。

我发现以这种方式编码（即引发错误并拯救它们）使我更容易分离实际的业务逻辑，并处理业务逻辑中出现的不同错误/情况。到目前为止，它有效，但我想就这是否是最佳做法或我没有按预期使用 begin/rescue 收集意见？

Answer 1

如果可以的话，将其干燥是一个好主意，也是 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 开发人员谈论测试驱动开发的好处时，这种方法的主要好处之一是它不太可能导致像您在这里得到的那样冗长而复杂的方法。您更有可能拥有更干燥的代码，以及更小、更简单的方法。

我还建议，一旦你完成了这个，你再看看并尝试进一步简化它。会有更多的简化空间，但我建议迭代地重构它，并按照我的描述将其分解并开始进行测试。

Answer 2

我将 Kevin Bedell 的回答与 Victor Moroz 的洞察力以及and and or in Ruby are flow control structures 的事实结合起来。我想出了这个：

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