2013-02-28 81 views
0

「高清顯示你會如何重構這個

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

+0

你需要什麼做不同?也許提供一些背景或動機。 – 2013-02-28 02:03:31

回答

0

對於初學者,您可以將所有的邏輯推送到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 
+0

謝謝!它的工作... – 2013-03-04 06:02:04

+0

我其實試圖重構它本身,但我沒有使用太多的模型,但感謝techniq – 2013-03-04 06:36:49

1

它在某些情況下,更快,更便宜的基於搜索的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) 
0

最好是處理對象集合,而不是對象屬性數組。因此@members應該是MemberUser對象的數組,而不是名稱的字符串數組。

這可能不是一個很好的回答你的問題,但我會按照由外而內發展的原則和目標是能夠得到你想要的東西是這樣的:

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過濾結果是最後的手段。