你会如何重构这个

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

对于初学者，您可以将所有的逻辑推送到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 2

它在某些情况下，更快，更便宜的基于搜索的id的表 - 这可能是这些案件之一。

@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 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 

在你的部门模型，设置范围/关联将返回适当的结果到上面使用的方法。使用Ruby过滤结果是最后的手段。