如何在 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

基于 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 3

只需创建一个 setter 辅助方法，如下所示：

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 4

如果查找所有嵌套的 if 和重复的逻辑有点混乱。请记住，您可以使用 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