Comment puis-je réduire la répétition dans ce code Ruby on Rails?

Question

Ceci est un extrait de code d'une méthode de mise à jour dans mon application. La méthode POST est un tableau d'ID utilisateur dans params [: assigned_ users_ list_id]

L'idée est de synchroniser les entrées d'associations de BD avec celles qui viennent d'être soumises, en supprimant les bonnes (celles qui existent dans le DB mais pas la liste) et en ajoutant les bons (vice versa).

@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 

Cela fonctionne - super!

Mes premières questions, sont ceux « si » et « à moins que des » déclarations totalement redondante, ou est-il prudent de les laisser en place?

Ma question suivante est, je veux répéter ce code exact immédiatement après, mais avec « souscrit » au lieu de « attribué » ... Pour y parvenir, je n'ai une trouvaille & remplacer dans mon éditeur de texte , me laissant avec presque ce code dans mon application deux fois. C'est à peine conforme au principe DRY! Pour être clair, toutes les occurrences des lettres «assignées» deviennent «souscrites». Il est passé params [: abonnés_liés_ ID_liste], et utilise @ task.subscribed_ users.delete user etc ...

Comment puis-je répéter ce code sans le répéter?

Merci comme d'habitude

Answer 1

Vous n'avez pas besoin si et à moins que des déclarations. En ce qui concerne la répétition, vous pouvez faire un tableau de hachages représentant ce dont vous avez besoin. Comme ceci:

[ 
    { :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 

Mes noms de variables ne sont pas le choix le plus heureux que vous puissiez les modifier pour améliorer la lisibilité.

Answer 2

Il me semble qu'il me manque quelque chose, mais ne faites-vous pas cela?

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