时间戳

首页 > 解决方案 > 重构冗余循环逻辑

ruby - 重构冗余循环逻辑

问题描述

我的代码应该从 QUIZ (MyQuiz) 中获取答案 (true)，并将它们与 Root 类型进行比较。测验问题具有通过 RootProfile 与它们关联的类型（名称）。例如，

question.root_profile.name

会返回类似：

{question: "1", name: "ocean", ...}

我也应该知道我有什么类型的答案。

answer.question.root_profile.name

当我从 MyQuiz 取回所有真实答案时，我可以拥有 10 个海洋 6 只鸟 3 个轮胎。然后，基于此，我计算每个测验的百分位数。如果测验有 100 个问题，它将计算 10% 的海洋、6% 的鸟类和 3% 的轮胎。

代码如下：

module RootServices
  class RootCount

    def initialize (my_quiz)
      @my_quiz = my_quiz
    end

    attr_reader :my_quiz

    def root_counts(root_profile)
      total = []
      all_answers.each do |answer|
        if answer.question.root_profile.name == root_profile.name
          total << answer
        end
      end

      total_percentage = (total.count.to_f / all_answers.count.to_f) * 100
      element = {root_profile_id: root_profile.id, root_profile: root_profile.name, total: total_percentage}
      return element
    end

    def root_branch
      root_branch_total = []

      # Pass root_profile object to root_counts and store in the root_branch_total array of hashes the returned element
      root_profiles.each do |root_profile|
        root_branch_total << root_counts(root_profile)
      end

      return root_branch_total
    end

    def root_profiles
      title = RootProfile.all
    end

    def all_answers
      all_answers = my_quiz.my_answers.where(answer: true)
    end

  end
end

它有效，但有点过于复杂，而且循环似乎是多余的。我想知道是否会有更优雅的方式。我的逻辑有效，但我的日志显示来自 Answers = true 的查询（假设我的测试有五个 true），其中五个查询针对 RootProfile，五个查询针对嵌套问题。这是一个视觉效果：

SELECT "my_answers".* FROM ...
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
SELECT "my_answers".* FROM ...
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
  SELECT  "questions".* FROM "questions"
  SELECT  "root_profiles".* FROM "root_profiles"
...
3x more

编辑：在下面添加重构代码

module RootServices
  class RootCount

    def initialize (my_quiz)
      @my_quiz = my_quiz
      @root_profiles = RootProfile.all
      @all_answers = @my_quiz.my_answers.where(answer: true)
    end    

    def root_branch
      root_profiles.map { |root_profile| root_counts(root_profile) }
    end

    private

      attr_reader :my_quiz, :root_profiles, :all_answers

      def root_counts(root_profile)
        answers = matching_answers(root_profile)
        {
          root_profile_id: root_profile.id,
          root_profile: root_profile.name,
          total: percentage_of_total(answers)
        }
      end

      def matching_answers(root_profile)
        all_answers.joins(question: :root_profile).where("root_profiles.name = ?", root_profile.name)
      end

      def percentage_of_total(answers)
        (answers.count.to_f / all_answers.count.to_f) * 100
      end

  end
end

标签： ruby

解决方案

首先，我认为您可能在 ruby​​ 中遗漏了几个核心概念 - 最明显的是隐式返回 - 在您的代码的情况下，这意味着您不需要return在方法末尾声明，而且您不需要不需要分配给单行中的局部变量。Ruby 就是这样。

您可以通过将要比较的所有名称收集到一个查询中来减少查询，而不是重复调用answer.question.root_profile.name. 此外，您可能应该在 SQL 中进行选择，而不是立即将其转换为数组。我将为您重构一些方法，以提供一个示例，说明如何更好地做到这一点..类似于以下内容

def root_counts
  answers = matching_answers(root_profile)
  {
    root_profile_id: root_profile.id,
    root_profile: root_profile.name,
    total: percentage_of_total(answers)
  }
end

private

def root_profiles
  RootProfile.all
end

def percentage_of_total(answers)
  (answers.count.to_f / true_answers.count.to_f) * 100
end

def true_answers
  my_quiz.my_answers.where(answer: true)
end

def matching_answers(root_profile)
  true_answers
    .joins(:questions, :root_profiles)
    .where("root_profiles.name = ?", root_profile.name)
end

我不完全理解你试图用你的root_branch方法做什么，因为我可以看到它返回一个数组，其中包含（1）数据库中所有对象的组合，和RootProfile（2）一个集合包含该百分比计数的哈希root_profile（由root_count方法返回）。我认为你应该重新考虑这个逻辑。

另外，我可能会考虑将root_profiles您使用类初始化的实例变量（也使用 an attr_reader） - 因为它加载整个root_profiles表，因此最好只调用一次

推荐阅读