这是重构的正确方法吗？有什么更好的我可以做的吗？

Question

我已经创建了一个Impressions 模型，它可以跟踪某张照片或某个事件有多少“浏览量”。它通过 remote_url、remote_ip 和 user_agent 跟踪。它按我想要的方式工作。

照片控制器：

def show

  @photo.impressions.create(:request_url => request.url, :controller_name => controller_name, :action_name => action_name,
                            :ip_address => request.remote_ip, :user_agent => request.user_agent)
....

还有我的事件控制器：

def show

  @event.impressions.create(:request_url => request.url, :controller_name => controller_name, :action_name => action_name,
                            :ip_address => request.remote_ip, :user_agent => request.user_agent)
....

但是，我正在考虑重构它（以使代码更清晰）。也许在Photo 和Event 模型中创建一个名为add_impression 的方法。它与上述完全相同，但封装了如何创建印象的功能。它看起来像：

照片控制器：

def show

  @photo.add_impression(request.url, controller_name, action_name, request.remote_ip, request.user_agent)
....

还有我的事件控制器：

def show

  @event.add_impression(request.url, controller_name, action_name, request.remote_ip, request.user_agent)
....

使用这种方法，我最终会在 Photo 和 Event 模型中使用 add_impression 方法。不完全干燥（不要重复自己）。我正在考虑创建一个Impression 模块来解决这个问题。

这是正确的方法吗？或者还有什么我应该做的吗？

PS 我还没有很好地调整模块。因此，如果有人想留下代码 sn-p 或指向值得阅读的文章的链接。请这样做。

Answer 1

我个人会让 Impression 负责使用类方法创建印象，例如：

class Impression < ActiveRecord::Base

  def self.add_impression impressionable, options
    impressionable.impressions.create options
  end

end

然后在控制器中：

Impression.add_impression @photo, options