我该如何干燥这种方法？

问题描述：

def restore_download_delete_file 
    begin 
     case params[:submit] 
     when "restore" 
     restore_status = restore_file(params[:file_names]) 
     raise if restore_status != 0 
     flash[:notice] = "File Successfully Restored." 
     redirect_to :action => "database_settings" 
     when "download" 
     download_status = download_file(params[:file_names]) 
     raise if download_status != 0 
     when "delete" 
     delete_status = delete_file(params[:file_names]) 
     raise if delete_status != 0 
     flash[:notice] = "File Successfully Deleted." 
     redirect_to :action => "database_settings" 
     end 
    rescue Exception => e 
     flash[:error] = "Error with #{params[:submit]}! Please retry." 
     redirect_to :action => "database_settings" 
    end 
    end

我该如何改进这种方法？我该如何干燥这种方法？

答

尝试这样

begin 
    status = send("#{params[:submit]}_file", params[:file_names]) 
    raise unless status == 0 
    if params[:submit] == 'restore' || params[:submit] == 'delete' 
    flash[:notice] = "File Successfully #{params[:submit].capitalize}d" 
    end 
rescue Exception => e 
    flash[:error] = "Error with #{params[:submit]}! Please retry." 
ensure 
    redirect_to :action => "database_settings" unless params[:submit] == 'download' 
end

它看起来不错，但我不想在任何页面上重定向下载场景。 – 2011-03-28 12:48:51

下载支票已添加重定向 – Ashish 2011-03-28 12:51:58

答

您可以通过将其分为四个进行清理：一个用于恢复，一个用于删除，一个用于下载，另一个用于调用合适的处理异常。

def restore_download_delete_file 
    begin 
    self.send "#{params[:submit]}" 
    rescue Exception => e 
    flash[:error] = "Error with #{params[:submit]}! Please retry." 
    redirect_to :action => "database_settings" 
    end 
end 

def restore 
    restore_status = restore_file(params[:file_names]) 
    raise if restore_status != 0 
    flash[:notice] = "File Successfully Restored." 
    redirect_to :action => "database_settings" 
end 

def download 
    download_status = download_file(params[:file_names]) 
    raise if download_status != 0 
end 

def delete 
    delete_status = delete_file(params[:file_names]) 
    raise if delete_status != 0 
    flash[:notice] = "File Successfully Deleted." 
    redirect_to :action => "database_settings" 
end

此外，几个注意事项：

提高适当的异常，而不是零。
不要拯救所有例外。代之以拯救那些你正在养的人。

您正在覆盖现有的方法名称，如restore_file，download_file等。 – Ashish 2011-03-28 12:25:00

@Ashish：现有的方法是什么？ – kikito 2011-03-28 12:28:05

在您的restore_file方法中，restore_file（params [：file_names]）调用存在。这意味着它已经在某个地方定义了。 – Ashish 2011-03-28 12:36:14

答

def restore_download_delete_file 
    submit = params[:submit] 
    if not restore_file(params[:file_names]).zero? 
     flash[:error] = "Error with #{submit}! Please retry." 
    elsif submit != "download" 
     flash[:notice] = "File Successfully #{submit.capitalize}d." 
    else 
     return 
    end 
    redirect_to :action => "database_settings" 
end

答

您可以随时将您的巨型法进一点解释：

private 
FILE_CMDS = { 
    'delete' => { 
     :action => lambda { |names| delete_file(names) }, 
     :notice => 'File Successfully Deleted.', 
     :redirect => 'database_settings', 
    }, 
    'download' => { 
     :action => lambda { |names| download_file(names) }, 
    }, 
    'restore' => { 
     :action => lamba { |names| restore_file(names) }, 
     :notice => 'File Successfully Restored.', 
     :redirect => 'database_settings', 
    }, 
} 

public 
def restore_download_delete_file 
    begin 
     cmd = FILE_CMDS[params[:submit]] 
     status = cmd[:action].call(params[:file_names]) 
     raise if(status != 0) 
     flash[:notice] = cmd[:notice] if(cmd[:notice]) 
     redirect_to :action => cmd[:redirect] if(cmd[:redirect]) 
    rescue Exception => e 
     flash[:error] = "Error with #{params[:submit]}! Please retry." 
     redirect_to :action => "database_settings" 
    end 
end

但我认为egarcia的做法是很有道理的;没有必要将所有这些东西混合到一个方法中。共同点确实非常小，因此一个控制器中的四个方法使得更有意义：一个动作，一个方法。 DRY只是一个指导方针，并不是不惜一切代价遵循教条。

我该如何干燥这种方法？

相关推荐