如何優化我的代碼

Question

我想優化我的代碼我如何？

如果我添加了新的角色，然後再添加一個條件，但我不想要。我想要任何沒有。角色，但我只需要一個條件是可能的？

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 

Answer 1

沒有真正測試了這一點，但我會打破這是這樣的：

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總是走來作爲角色的用戶是複數形式所以藝術家意味着「藝術家」角色。

當然這假定所有角色都是有效的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 

Answer 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 

事實上，我甚至創建用戶專用範圍處理邏輯來獲得根據該組的正確的用戶。

Answer 3

我認爲下面是你需要的答案：

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 

Answer 4

在作爲參數傳遞

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 

這將在兩個第一條件下工作的作用

在最後我猜測，用戶都有「fa ns「和」藝術家「角色。然後通過user.role.first