2009-08-03 89 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])