0

コードを最適化したい どうすればよいですか?

新しい役割を追加した場合、もう1つの条件が追加されますが、それは望ましくありません。いいえ欲しいです。の役割ですが、必要な条件は 1 つだけですか?

after_save :announcement_send

def announcement_send
  if self.send_now == true && self.group_id.to_s == "Artists"
    User.having_role("artist").each do |user|
      ArtistMailer.announcement_user(self, user).deliver
    end
  elsif self.send_now == true && self.group_id.to_s == "Fans"
    User.having_role("fan").each do |user|
      ArtistMailer.announcement_user(self, user).deliver
    end
  elsif self.send_now == true && self.group_id.to_s == "Both"
     User.not_having_role("admin").each do |user|
       ArtistMailer.announcement_user(self, user).deliver
     end
  end        
end
4

4 に答える 4

3

これを実際にテストしたわけではありませんが、次のように分類します。

def role(s)                                                                                                                                                                               
  if s == "Both"                                                                                                                                                                          
    User.not_having_role("admin")                                                                                                                                                         
  else                                                                                                                                                                                    
    User.having_role(s.singularize.downcase)                                                                                                                                              
  end                                                                                                                                                                                     
end         

if self.send_now                                                                                                                                                                          
  role(self.group_id.to_s).each do |user|                                                                                                                                                 
    ArtistMailer.announcement_user(self, user).deliver                                                                                                                                    
  end                                                                                                                                                                                     
end

このコードは、group_id が常にユーザーのロールの複数形として表示されることを前提としています。したがって、Artists は「アーティスト」ロールを意味します。

もちろん、これはすべてのロールが有効な group_id であることを前提としています。そうでない場合は、 group_id を内部の可能な値のホワイトリストと照合して確認できますdef role。

white_list = ['artist', 'fan']
role = s.singularize.downcase
if white_list.include? role
  User.having_role(role)
else
  # possibly throw an exception
end
于 2012-12-04T11:30:12.897 に答える
2

良いコードはストーリーを語るべきです。

WHITELIST = %w(artist fan)

def announcement_send
  return unless send_now
  users_list_for_announcement.each { |user| notify(user) }
end

def users_list_for_announcement
  in_whitelist? = ->(group) { WHITELIST.include?(group) }
  case formatted_group_name
  when "both"        then User.not_having_role("admin")
  when in_whitelist? then User.having_role(formatted_group_name)
  else []
  end
end

def formatted_group_name
  group_id.to_s.singularize.downcase
end

def notify(user)
  ArtistMailer.announcement_user(self, user).deliver
end

実際には、グループに応じて適切なユーザーを取得するロジックを処理するユーザーに専用のスコープを作成することさえあります。

于 2012-12-04T11:33:25.633 に答える
0

以下があなたの必要な答えだと思います：


if self.send_now
if self.group_id.to_s == "Both" 
    User.not_having_role("admin").each do |user|
        ArtistMailer.announcement_user(self, user).deliver
    end
elsif self.group_id.to_s
    User.having_role(self.group_id.to_s.singularize.downcase).each do |user|
        ArtistMailer.announcement_user(self, user).deliver
    end     
end
end
于 2012-12-04T11:37:19.463 に答える
0

ロールを引数として渡す

def announcement_send(role)
  if self.send_now == true 
    User.having_role(role).each do |user|
      ArtistMailer.announcement_user(self, user).deliver
    end  
end

def after_save(role)
   announcement_send(role)
end

これは、最初の 2 つの条件で機能します。

最後に、ユーザーには「ファン」と「アーティスト」の両方の役割があると推測しています。次に、user.role.first を渡します

于 2012-12-04T11:41:34.117 に答える