1

現在、モデルの URL スラッグを動的に生成しています (そして、それらを解釈するために to_param/self.from_param を実装しています)。私のスラッグ生成コードは冗長に感じ、リファクタリングを使用できます。

これをどのようにリファクタリングすれば、まだ読みやすくなりますが、冗長性が減り、おそらくより明確になりますか?

関係

ユーザーhas_many :lists

所有者のリストの所属先:所有者

コード

def generate_slug
  if self.owner
    slug_found = false
    count      = 0
    temp_slug  = to_slug

    until slug_found
      # increment the count
      count += 1

      # create a potential slug
      temp_slug = if count > 1
        suffix = "_" + count.to_s
        to_slug + suffix
      else
        to_slug
      end

      # fetch an existing slug for this list's owner's lists
      # (i.e. owner has many lists and list slugs should be unique per owner)
      existing = self.owner.lists.from_param(temp_slug)

      # if it doesn't exist, or it exists but is the current list, slug found!
      if existing.nil? or (existing == self)
        slug_found = true
      end
    end

    # set the slug
    self.slug = temp_slug
  else
    Rails.logger.debug "List (id: #{self.id}, slug: #{self.slug}) doesn't have an owner set!"
  end
end
4

1 に答える 1

1

あなたは多分これを行うことができます

def generate_slug
  return Rails.logger.debug "List (id: #{self.id}, slug: #{self.slug}) doesn't have an owner set!" if !self.owner

  count = 1
  begin
    temp_slug = %Q!#{to_slug}#{"_#{count}" if count > 1}!
    existing = self.owner.lists.from_param(temp_slug)

    if existing.nil? or (existing == self)
      self.slug = temp_slug
    end
  end while count += 1
end

しかし、2つのことがあります。まず、良くない無限ループがあります。次に、オブジェクトが存在し、サフィックスを増やす必要があるかどうかを毎回チェックするためにループする代わりに、最後の既存のリストを取得し、その後に1つだけ追加する方がよいでしょう。

于 2012-10-11T03:56:07.480 に答える