2009-08-03 12 views
1

これは、アプリケーションのアップデートメソッドのコードスニペットです。このメソッドは、ユーザIDの配列をparams [:assigned_ users_list_id]にPOSTします。このRuby on Railsコードの繰り返しを減らすにはどうすればよいですか?

考えられるのは、DBアソシエーションのエントリを、サブミットされたものと同期させることです。 DBではなくリスト)、正しいものを追加する(逆も同様)。

@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

のようにしている場合としない限り、文は必要ありません。 繰り返しについては、必要なものを表すハッシュの配列を作ることができます。このよう :

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

+0

@Senad Uka、本当にコミュニティのwikiにはこの回答はありません。担当者に相談してください。 –

+0

私は本当に評判について気にしません。誰かが自分のコードを改善すれば好きです。 –

1

私はここに何か不足しているようですが、これをやっているだけではありませんか?

@task.assigned_users = User.find(params[:assigned_users_list_id]) 
関連する問題