Come posso ridurre la ripetizione in questo codice Ruby on Rails?

Question

Questo è un frammento di codice da un metodo di aggiornamento nella mia applicazione. Il metodo è POSTed un array di ID utente in params [: assegnato_ utenti_ elenco_id]

L'idea è di sincronizzare le voci delle associazioni DB con quelle appena inviate, rimuovendo quelle giuste (quelle esistenti nel DB ma non l'elenco) e aggiungendo quelle giuste (viceversa).

    @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

Funziona alla grande!

La mia prima domanda è: quelle affermazioni 'if' e 'salvo' totalmente sono ridondanti o è prudente lasciarle sul posto?

La mia prossima domanda è, voglio ripetere questo codice esatto immediatamente dopo questo, ma con 'iscritto' al posto di 'assegnato' ... Per raggiungere questo obiettivo ho appena fatto una ricerca & amp; sostituisco nel mio editor di testo, lasciandomi con quasi questo codice nella mia app due volte. Questo non è in linea con il principio DRY!

Giusto per essere chiari, ogni istanza delle lettere "assegnate" diventa "iscritta". Viene passato params [: subsigned_ users_ list_ id] e usa @ task.subscribed_ users.delete user etc ...

Come posso ripetere questo codice senza ripeterlo?

Grazie come al solito

Answer 1

Non è necessario se e a meno dichiarazioni. Per quanto riguarda la ripetizione, puoi creare una serie di hash che rappresentano ciò di cui hai bisogno. In questo modo:

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

I miei nomi di variabili non sono la scelta più felice, quindi puoi cambiarli per migliorare la leggibilità.

Answer 2

Mi sembra che manchi qualcosa qui, ma non lo fai e basta?

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