1

I'm currently generating url slugs dynamically for my models (and implementing to_param/self.from_param to interpret them). My slug generation code feels verbose, and could use a refactor.

How would you refactor this so that it is still readable, but less verbose and perhaps more clear?

Relationships

User has_many :lists

List belongs_to :owner

Code

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

但是有两件事。首先，您有一个不好的无限循环。其次，不是每次循环检查对象是否存在以及您需要增加后缀，您最好获取最后一个现有列表并在其后添加一个。

于 2012-10-11T03:56:07.480 回答