Rails：清理凌乱的控制器方法

Question

我有一个带有很多重定向条件的长控制器方法：

def show
  get_param_user
  if params[:id].match(/\D/)
    @document = Document.where(:user_id => @user.id, :issue => params[:id]).first
  else
    @document = Document.find(params[:id])
  end
  unless @document.blank?
    unless @document.template.name == "Media"
      unless @document.retired?
        @creator = User.find(@document.user)
        if @creator == @user # if document exists, based on name and id
          @document.components.each do |a|
            redirect_to share_error_url, :flash => { :error =>  "#{@document.title} contains retired content and is now unavailable." } if a.retired? and return
          end
          render @document.template.name.downcase.parameterize.underscore
        end
      else # if retired
        redirect_to share_error_url, :flash => { :error =>  "That document has expired." } and return
      end
    else # if media
      redirect_to share_error_url, :flash => { :error =>  "Media has no public link." } and return
    end
  else # if document doesn't exist
    redirect_to share_error_url, :flash => { :error =>  "Can't find that document. Maybe check your link. Or maybe it was deleted. Ask #{@user.name}." } and return
  end
end

正如您可能猜到的那样，在某些情况下它很容易出错。有没有更简洁的方法来重写它以使其更健壮？我知道每个方法应该只有一个render 或redirect_to，但我不知道还有什么方法可以实现我需要的。

谢谢！

Answer 1

一些具体的小事。

首先，一般来说，最好不要将until 与else 条件一起使用，如果可以使用if 则更少：

unless @document.blank?

与

相同

if @document.present?

第二，你用

    @creator = User.find(@document.user)

通常你可以简单地使用：

    @creator = @document.user

语义有点不同（在第一种情况下，如果@document.user 是nil，你会立即得到一个异常，在第二种情况下不会），但第二种情况是你通常需要的。

第三，如果合理的话，你可以将代码从控制器移到模型中，并使用一些不错的枚举器：

def has_retired_components?
  @document.components.any?(&:retired?)
end

此外，您的控制器方法并不那么复杂。只是

if @document.present? and @document.showable? # also @document.try(:showable?)
  render whatever
else
  redirect_to error_url, flash: { error: error_message }
end

error_message 可能是方法调用的结果（如果有意义，则在对象本身上）。这样，您可以移动逻辑以验证对象是否可以在其他不太受渲染逻辑混淆的地方显示。

问题在于，如果您有一个showable? 方法和另一个显示错误消息的方法，您必须确保两者的业务逻辑始终正确。一种选择是将其与验证的工作方式类似地对待：有一个方法（让我们用可怕的名称 showable_validation 来调用它），它返回带有错误和消息的哈希（对象无法显示的原因，例如{title: 'this is an error message'}。showable? 方法将是：

def showable?
  showable_validation.empty?
end

然后你在模型中也会有类似的东西：

def showable_error
  showable_validation.values.first
end

那就是error_message (@document.showable_error)。这样一来，逻辑就只有一种方法了。