ruby-on-rails - 你会如何重构这个

Question

'定义显示

 @dept = Dept.find(params[:id])
    @members = @dept.members_list.collect{|a|a.name}
    dif = @dept.users.collect{|a|[a.name,a.id]}
    @admin_list = @dept.admin_users.collect{|a|a.user}
   @not_member_users = User.all_users - dif
   @not_admin_user = (@dept.list_of_deptmembers - @dept.admin_users).collect{|a|[a.user.name, a.id]}'    

我如何重构@not_admin_user？

Answer 1

在某些情况下，搜索基于id's 的表会更快、更便宜——这可能是其中一种情况。

@dept = Dept.find(params[:id])

# assuming both `members_list` and `admin_users` actually point to `User` objects
member_ids = @dept.members_list.collect(&:id)
admin_ids = @dept.admin_users.collect(&:id)
non_admin_id = member_ids - admin_ids

non_admin_users = User.where("id in ?", non_admin_id)

Answer 2

对于初学者，您可以将所有这些逻辑下推到 Dept 模型中。

def show
  @dept = Dept.find(params[:id])
  @admin_list = @dept.admin_list
  @members = @dept.members
  @not_member_users = @dept.not_member_users
  @not_admin_user = @dept.not_admin_user
end

class Dept < ActiveRecord::Base
  def members
    self.members_list.collect{ |a| a.name }
  end

  def admin_list
    self.admin_users.collect{ |a| a.user }
  end

  def not_member_users(user = User)
    dif = self.users.collect{|a|[a.name,a.id]}
    user.all_users - dif
  end

  def not_admin_user
    (self.list_of_deptmembers - self.admin_users).collect{|a|[a.user.name, a.id]}
  end
end

Answer 3

最好处理对象的集合，而不是对象的属性数组。所以@members应该是一个Member或User对象的数组，而不是名称的字符串数组。

对于你的问题，这可能不是一个很好的答案，但我会遵循由外而内开发的原则，旨在能够像这样得到你想要的：

def show
  @dept = Dept.find(params[:id])
  @members          = @dept.members
  @admin_list       = @dept.admin_users
  @not_member_users = @dept.non_member_users
  @not_admin_users  = @dept.non_admin_users
end

在您的 Dept 模型中，设置范围/关联，以便将适当的结果返回给上面使用的方法。使用 Ruby 过滤结果是最后的手段。