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

只需创建一个 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 2

基于 Maxim 的想法，但注意到您的权限是分层的（即 "full" 表示编辑和查看，"edit" 表示查看），我会将您的辅助方法压缩为：

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

首先，您可能想看看使用 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

就我个人而言，我发现通过应用 "Don't Repeat Yourself" (DRY) 原则来减少代码中的重复通常会暴露底层结构，并且更容易将其重塑为更简洁和灵活的内容。

例如，这里有一些针对 resource.ed_level != 'group' 的测试，由于处于测试的 else 块中，断言相反，所以没有办法永远不会是案例.

Answer 4

If 发现所有嵌套的 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

如何用 Ruby 中的多个布尔变量重构方法

How to refactor method with multiple boolean variables in Ruby

ruby

refactoring

ruby-on-rails

code-cleanup