1

我有这个创建方法：

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 方法的文字复制粘贴。

我有两个问题：

  1. 有什么办法让我把它擦干吗？
  2. 这对于一般的控制器来说是一个好的模式吗？

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

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

4

3 回答 3

4

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

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

于 2012-07-02T13:39:17.297 回答
1

and我将 Kevin Bedell 的回答与 Victor Moroz 的洞察力结合起来，以及orRuby 中的 和 是流控制结构这一事实。我想出了这个：

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，但这是完全不同的对话。

于 2012-07-02T18:45:51.660 回答
-2

如果它仍然 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
于 2012-07-02T12:36:26.223 回答