如何在Rails/Ruby中处理琐碎的"重复代码"气味

Question

因此,我们都努力减少重复(DRY)和其他气味,并尽可能保持我们的代码良好和干净.对于Ruby代码,有很多仪器可以检测气味,例如相当不错的Calibre服务.

但是,似乎我对代码重复的定义与工具不同.我认为这可能与Ruby的做事方式有关,你几乎从不直接访问变量,而是通过方法调用.考虑一下Rails控制器的这个片段:

def update_site_settings
  SiteSettings.site_name = params[:site_name]
  SiteSettings.site_theme = params[:site_theme]
  expire_fragment('layout_header')
  flash[:notice] = t(:Site_settings_updated)
  redirect_to :controller => 'application', :action => 'edit_site_settings'
end

由于对"params"方法的两次调用,因此标记了代码重复警告.所以我的问题是,分配params到局部变量真的会有所改进吗？我认为编写它的方式是最清晰简洁的方法,事实上,params方法而不是变量是Ruby中"做生意的成本".

我看错了吗？

编辑:在这种情况下,更漂亮的方式可能是进行SiteSettings.update_attributes(params)样式更新.如果您愿意,请考虑另一个代码段中的相同问题:

def update
  @mailing_list = MailingList.find(params[:id])

  if @mailing_list.update_attributes(params[:mailing_list])
    flash[:notice] = t:Mailing_list_updated
    redirect_to(mailing_lists_path)
    ...

Answer 1

我想你也可以声明一个小实例Feature Envy代码味道的小实例，尽管它确实有点微不足道。

也许可以引入 MailingList 上的类方法，因此控制器方法变成类似

def update
  if @mailing_list = MailingList.update_attributes_by_id(params)
    ...

class MailingList
  def self.update_attributes_by_id(params)
     id = params.delete(:id)
     find(id).update_attributes(params)
     ...

（未经测试，因此请小心处理）

现实生活中我会打扰吗？可能不是，一方面，由两部分组成的查找/更新非常常见，人们会立即理解它 - 即使您有一个完美表达的名称，有人使用上面的代码也必须停下来思考一下。

这些分析器（我在自己的代码上运行 Kevin Rutherford 的臭味）很棒，但它们不理解上下文，因此它们很少会提供完美的信息。它们对于识别可能受益于关注的领域很有用，但会有很多误报，而且它们也会错过一些东西，因此需要有意识地使用它们。