ruby-on-rails - create_unique メソッドで複雑なロジックをリファクタリングする方法は?

Question

この複雑なロジックを単純化して、一意の Track オブジェクトを作成したいと思います。

def self.create_unique(p)
  f = Track.find :first, :conditions => ['user_id = ? AND target_id = ? AND target_type = ?', p[:user_id], p[:target_id], p[:target_type]]
  x = ((p[:target_type] == 'User') and (p[:user_id] == p[:target_id]))
  Track.create(p) if (!f and !x)
end

Answer 1

これは、いくつかの単純な抽出メソッドを使用して を書き直したものです。

def self.create_unique(attributes)
  return if exists_for_user_and_target?(attributes)
  return if user_is_target?(attributes)

  create(attributes)
end

def self.exists_for_user_and_target?(attributes)
  exists?(attributes.slice(:user_id, :target_id, :target_type))
end

def self.user_is_target?(attributes)
  attributes[:target_type] == 'User' && attributes[:user_id] == attributes[:target_id]
end

この書き直しは、意図を説明するのに役立つ小さくて説明的な方法に対する私の好みを示しています。また、次のような場合にガード句を使用するのも好きcreate_uniqueです。ハッピー パスは最後の行 ( create(attributes)) で明らかにされていますが、警備員は例外的なケースを明確に説明しています。Rails 3 を前提としていますが、私のexists?inの使用はexists_for_user_and_target?の良い代替になると思います。find :first

代わりに一意性アクティブ モデル検証の使用を検討することもできます。

Answer 2

@@keys = [:user_id, :target_id, :target_type]
def self.create_unique(p)
  return if Track.find :first, :conditions => [
    @@keys.map{|k| "#{k} = ?"}.join(" and "),
    *@@keys.map{|k| p[k]}
  ]
  return if p[@@keys[0]] == p[@@keys[1]]
  return if p[@@keys[2]] == "User"
  Track.create(p)
end