2009-08-03 134 views
1

這是我的應用程序中更新方法的一段代碼。該方法在params [:assigned_user_list_id]中發佈用戶標識的數組如何減少此Ruby on Rails代碼中的重複?

這個想法是通過刪除正確提交的數據庫來將數據庫關聯條目與那些剛剛提交的數據庫條目同步(存在於數據庫但不是列表)並添加正確的(反之亦然)。

@list_assigned_users = User.find(:all, :conditions => { :id => params[:assigned_users_list_id]}) 
    @assigned_users_to_remove = @task.assigned_users - @list_assigned_users 
    @assigned_users_to_add = @list_assigned_users - @task.assigned_users 

    @assigned_users_to_add.each do |user| 
     unless @task.assigned_users.include?(user) 
      @task.assigned_users << user 
     end 
    end 
    @assigned_users_to_remove.each do |user| 
     if @task.assigned_users.include?(user) 
      @task.assigned_users.delete user 
     end 
    end 

它的作品 - 太棒了!

我的第一個問題是,那些「如果」和「除非」語句完全多餘的,或者是謹慎的做法是讓他們在的地方?

我的下一個問題是,我想在此之後,立即重複此確切代碼,但在地方的「分配」訂閱'......爲了實現這一點,我只是做了找到&替換我的文字編輯器,讓我幾乎在我的應用程序中的這個代碼兩次。這與DRY校長很難保持一致!

只要是明確的,「分配」字母的每個實例變成「訂閱」。它傳遞PARAMS [:subscribed_ users_ list_ ID],並使用@ task.subscribed_ users.delete用戶等等

我怎麼能重複這個代碼,而無需重複呢?

感謝像往常一樣

回答

2

你不需要if和unless語句。 至於重複,你可以做一些代表你所需要的散列。 像這樣:

[ 
    { :where_clause => params[:assigned_users_list_id], :user_list => @task.assigned_users} , 
    { :where_clause => params[:subscribed_users_list_id], :user_list => @task.subscribed_users} 
    ] each do |list| 
     @list_users = User.find(:all, :conditions => { :id => list[:where_clause] }) 
     @users_to_remove = list[:user_list] - @list_users 
     @users_to_add = @list_users - list[:user_list] 

     @users_to_add.each do |user| 
      list[:user_list] << user 
     end 
     @users_to_remove.each do |user| 
      list[:user_list].delete user 
     end 
     end 

我的變量名是不是最幸福的選擇,這樣你就可以改變它們以提高可讀性。

+0

這就是一些優秀的代碼!非常感謝你的回答。 – doctororange 2009-08-03 11:11:59

1

我似乎失去了一些東西,但你不只是這樣做呢?

@task.assigned_users = User.find(params[:assigned_users_list_id])