这是重构的正确方法吗？我还能做点更好的事吗？

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 方法。不完全是 DRY（不要重复自己）。我正在考虑创建一个 Impression 模块来解决这个问题。

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

PS 我还没有很好地调整模块。因此，如果有人愿意留下代码片段或值得阅读的文章链接。请这样做。

Answer 1

就我个人而言，我会让 Impression 负责使用类方法创建印象，如下所示：

class Impression < ActiveRecord::Base

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

end

然后在控制器中：

Impression.add_impression @photo, options