Как я могу уменьшить количество повторений в этом коде Ruby on Rails? - PullRequest
1 голос
/ 03 августа 2009

Это фрагмент кода из метода обновления в моем приложении. Метод представляет собой массив идентификаторов пользователей в параметрах [: assign_ users_ list_ id]

Идея состоит в том, чтобы синхронизировать записи ассоциаций БД с только что отправленными, удалив правильные (те, которые существуют в БД, но не в списке) и добавив правильные (наоборот).

    @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 [: subsed_ users_ list_ id] и использует @ task.subscribeed_ users.delete user и т. Д. ...

Как я могу повторить этот код, не повторяя его?

Спасибо как обычно

Ответы [ 2 ]

2 голосов
/ 03 августа 2009

Вам не нужно, если и если заявления. Что касается повторения, вы можете сделать массив хэшей, представляющих то, что вам нужно. Как это:

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

Мои имена переменных - не самый удачный выбор, поэтому вы можете изменить их, чтобы улучшить читаемость.

1 голос
/ 04 августа 2009

Я, кажется, что-то здесь упускаю, но разве вы просто не делаете это?

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