如何用Ruby中的多個布爾變量重構方法

Question

我在ruby中有一個設置幾個實例變量的方法，有條件地，我想知道如何重構它來清理它並使其不那麼冗長。我的第一個嘗試是將不同的條件分解爲多個較小的幫助器方法，但我不確定這是否是正確的解決方法。任何建議都會有幫助。

def admin_view 
    if resource.present? 
     if resource.ed_level == 'group' 
     if current_user && (current_user.admin || resource.admins_byemail.include?(current_user.email)) 
      @admin_full = true 
      @admin_edit = true 
      @admin_view = true 
     else 
      @admin_full = false 
      @admin_edit = false 
      @admin_view = false 
     end 
     else 
     if current_user && (current_user.admin || resource.admin_email_list('view').include?(current_user.email.downcase)) 
      if current_user.admin || (resource.admin_email_list('full').include?(current_user.email.downcase) && resource.ed_level != 'group') 
      @admin_full = true 
      @admin_edit = true 
      @admin_view = true 
      elsif resource.admin_email_list('edit').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      @admin_full = false 
      @admin_edit = true 
      @admin_view = true 
      elsif resource.admin_email_list('view').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      @admin_full = false 
      @admin_edit = false 
      @admin_view = true 
      end 
     else 
      @admin_full = false 
      @admin_edit = false 
      @admin_view = false 
     end 
     end 
    else 
     redirect_to school_missing_path 
    end 
    end 

根據下面的答案，我更新了我的代碼，如下所示。

def admin_view 
    if resource.present? 
     if resource.ed_level == 'group' 
     if current_user && (current_user.admin || resource.admins_byemail.include?(current_user.email)) 
      set_admin_permissions(full: true, edit: true, view: true) 
     else 
      set_admin_permissions(full: false, edit: false, view: false) 
     end 
     else 
     if current_user && (current_user.admin || resource.admin_email_list('view').include?(current_user.email.downcase)) 
      if current_user.admin || (resource.admin_email_list('full').include?(current_user.email.downcase) && resource.ed_level != 'group') 
      set_admin_permissions(full: true, edit: true, view: true) 
      elsif resource.admin_email_list('edit').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      set_admin_permissions(full: false, edit: true, view: true) 
      elsif resource.admin_email_list('view').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      set_admin_permissions(full: false, edit: false, view: true) 
      end 
     else 
      set_admin_permissions(full: false, edit: false, view: false) 
     end 
     end 
    else 
     redirect_to school_missing_path 
    end 
    end 

    private 

    def set_admin_permissions(full:, edit:, view:) 
    @admin_full = full 
    @admin_edit = edit 
    @admin_view = view 
    end 

Answer 1

首先，您可能想看看使用CanCanCan來正確地封裝您的權限。這是在您的控制器和視圖代碼中定義訪問限制和測試的更正式方式。

def admin_permissions 
    return [ ] unless resource.present? 

    case resource.ed_level 
    when 'group' 
    if current_user && (current_user.admin || resource.admins_byemail.include?(current_user.email)) 
     [ :full, :edit, :view ] 
    else 
     [ ] 
    end 
    else 
    email = current_user && current_user.email.downcase 

    if current_user && (current_user.admin || resource.admin_email_list('view').include?(email)) 
     if current_user.admin || resource.admin_email_list('full').include?(email) 
     [ :full, :edit, :view ] 
     elsif resource.admin_email_list('edit').include?(email) 
     [ :edit, :view ] 
     elsif resource.admin_email_list('view').include?(email) 
     [ :view] 
     end 
    else 
     [ ] 
    end 
    end 
end 

然後用這個像這樣：

@admin_privs = admin_permissions 

定義一些輔助方法，這樣

話雖這麼說，你可以顯着，如果你組織你的代碼有點不同歸結代碼：

def admin_full? 
    @admin_privs and admin_privs.include?(:full) 
end 

def admin_edit? 
    @admin_privs and admin_privs.include?(:edit) 
end 

def admin_view? 
    @admin_privs and admin_privs.include?(:view) 
end 

我個人發現，通過應用程序減少代碼中的重複撒謊「不要重複自己」（DRY）原則通常會暴露底層結構，並使其更容易重構成更簡潔和靈活的內容。

例如，當resource.ed_level != 'group'由於處於else塊的測試而斷言相反時，沒有辦法永遠不會出現這種情況，因此在這裏進行了大量測試。

Answer 2

只是做一個二傳手的輔助方法，像這樣：

def admin_view 
    if resource.present? 
    if resource.ed_level == 'group' 
     if current_user && (current_user.admin || resource.admins_byemail.include?(current_user.email)) 
     set_values(true, true, true) 
     else 
     set_values(false, false, false) 
     end 
    else 
     if current_user && (current_user.admin || resource.admin_email_list('view').include?(current_user.email.downcase)) 
     if current_user.admin || (resource.admin_email_list('full').include?(current_user.email.downcase) && resource.ed_level != 'group') 
      set_values(true, true, true) 
     elsif resource.admin_email_list('edit').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      set_values(false, true, true) 
     elsif resource.admin_email_list('view').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      set_values(false, false, true) 
     end 
     else 
     set_values(false, false, false) 
     end 
    end 
    else 
    redirect_to school_missing_path 
    end 
end 

def set_values(full, edit, view) 
    @admin_full = full 
    @admin_edit = edit 
    @admin_view = view 
end 

Answer 3

大廈關閉Maxim的想法，但注意到，您的權限層級（即「全」意味着編輯&視圖和「編輯」意味着視圖），我會凝結在輔助方法是：

def set_access_level(level) 
    case level 
    when :full 
    @admin_full, @admin_edit, @admin_view = true, true, true 
    when :edit 
    @admin_full, @admin_edit, @admin_view = false, true, true 
    when :view 
    @admin_full, @admin_edit, @admin_view = false, false, true 
    else 
    @admin_full, @admin_edit, @admin_view = false, false, false 
    end 
end 

然後你的代碼變成：

def admin_view 
    if resource.present? 
    if resource.ed_level == 'group' 
     if current_user && (current_user.admin || resource.admins_byemail.include?(current_user.email)) 
     set_access_level(:full) 
     else 
     set_access_level(:none) 
     end 
    else 
     if current_user && (current_user.admin || resource.admin_email_list('view').include?(current_user.email.downcase)) 
     if current_user.admin || (resource.admin_email_list('full').include?(current_user.email.downcase) && resource.ed_level != 'group') 
      set_access_level(:full) 
     elsif resource.admin_email_list('edit').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      set_access_level(:edit) 
     elsif resource.admin_email_list('view').include?(current_user.email.downcase) && resource.ed_level != 'group' 
      set_access_level(:view) 
     end 
     else 
     set_access_level(:none) 
     end 
    end 
    else 
    redirect_to school_missing_path 
    end 
end 

Answer 4

如果找到所有嵌套的ifs和重複的邏輯有點混亂。請記住，您可以使用return語句來使代碼更乾淨。我不能保證下面的邏輯正是你所追求的，但是在我看來，結構更具可讀性。

def admin_view 
    redirect_to school_missing_path unless resource.present? 
    access_level = calc_access_level 

end 

def calc_access_level 

    return :none unless resource.present? 
    return :none unless current_user 
    return :full if current_user.admin 

    email_raw = current_user.email 
    email = email_raw.downcase 

    if (resource.ed_level == 'group') 
     return resource.admins_byemail.include?(email_raw) ? :full, :none 
    end 

    ['view','full','edit'].each do |access_level| 
     if resource.admin_email_list(access_level).include?(email) 
     return access_level.to_sym 
     end 
    end 

    return :none 

end 

def set_access_level(level) 

    @admin_full, @admin_edit, @admin_view = false, false, false 
    case level 
    when :full 
    @admin_full, @admin_edit, @admin_view = true, true, true 
    when :edit 
    @admin_edit, @admin_view = true, true 
    when :view 
    @admin_view = true 
    end 
end