有没有办法在 ruby 中重构以下代码答案

【问题标题】：Is there a way to Refactor the following code in ruby有没有办法在 ruby 中重构以下代码
【发布时间】：2019-11-20 15:22:00
【问题描述】：

我写了一段代码，需要重构它。

Reviewing.where(reviewing_status_condition(employee_ids)).group(:employee_id).count.map{ 
|employee_id, reviewings_count_per_employee|
    employee_id if reviewings_count_per_employee >= @cycle.min_required_anon_feedback
  }.compact

有人可以帮忙吗？

【问题讨论】：

你想通过这次重构达到什么目的？
降低每个map造成的时间复杂度，并compact在一起
你在重构之前测试过这段代码并确保它确实有效吗？我可以想象.count.map 是有问题的，因为.count 创建了一个计数查询并返回一个整数 - 但是你在它上面调用 map 就像它是一个集合一样。

标签： ruby ruby-on-rails-5

【解决方案1】：

除了在 Ruby 中解决这个问题，您还可以选择让您的数据库来处理这个问题。但是要做到这一点，您首先需要知道您的查询将是什么，我会选择类似的内容：

SELECT reviewings.employee_id
FROM reviewings
WHERE ...
GROUP BY reviewings.employee_id
HAVING COUNT(reviewings.id) >= <your value>

这可以通过以下代码实现：

reviewings = Reviewing.arel_table

employee_ids = Reviewing
               .where(reviewing_status_condition(employee_ids))
               .group(:employee_id)
               .having(reviewings[:id].count.gteq(@cycle.min_required_anon_feedback))
               .pluck(:employee_id)

通过限制从数据库返回的数据，您可以省去 Ruby 端的数据操作。

如果您想了解更多关于 arel 的信息，我建议您查看他们的readme。

让我添加一个有趣的比较。假设您要订购一些比萨饼。

您目前正在执行以下操作：您致电您的披萨店并要求他们每份披萨都送一份。当比萨饼到达您家时，您和您的同伴需要在吃它们之前弄清楚要保留哪些比萨饼以及要丢弃哪些比萨饼。
我的建议是：查看菜单并预先确定您需要哪些比萨饼。只订购您需要的东西。当比萨饼到达您家时，所有决定都已经做出并处理（假设交付正确），您可以简单地享用您的比萨饼。

【讨论】：

有没有可能宁愿减少 map 的使用，然后只用一种可以解决问题的枚举器方法进行压缩？
@MaheshMesta 我在 Ruby 中添加了 another answer 来解决这个问题。
好答案。花时间尝试修复 Ruby 中的迭代很可能完全是浪费时间，因为问题实际上是首先将所有数据从数据库中提取出来。

【解决方案2】：

假设您无法像my other answer 中建议的那样更改数据供应。我会将您的 .map{...}.compact 更改为 .select{...}.keys，因为它可以更好地传达您想要通过代码实现的目标。您正在根据审核计数过滤/选择某些员工 ID。

reviewings_count_per_employee = Reviewing.where(reviewing_status_condition(employee_ids)).group(:employee_id).count

employee_ids = reviewings_count_per_employee
               .select { |_id, count| count >= @cycle.min_required_anon_feedback }
               .keys

我不确定这是否会降低时间复杂度，但它肯定会提高可读性。只要您处于事物的线性领域，我就不会担心时间复杂度。如果select 是O(N) 并且keys 是O(N)，那么你最终会得到O(2N)，这不是一个真正的问题。如果您想保留您的代码O(N)，则可以使用以下方法。

employee_ids = []
reviewings_count_per_employee.each do |id, count| 
  employee_ids << id if count >= @cycle.min_required_anon_feedback
end

不过，我个人认为select 的解决方案比each 的解决方案更具可读性。尽管这可能会节省一些 CPU 负载，但当您重新访问代码时，您的大脑必须更加努力地理解您在 6-12 个月内编写的内容。

【讨论】：

【解决方案3】：

对我来说，第一步是将where 子句和group 移动到命名范围。这将减少初始段：

Reviewing.where(reviewing_status_condition(employee_ids)).group(:employee_id).count

类似于：

Reviewing.reviewing_count(reviewing_status_condition(employee_ids))

这不是很短，但确实显着改善了实现与执行的分离。

第二步取决于您使用的 Ruby 版本。在 2.7 之前，执行 map 和 compact 步骤的正确方法是您已经或可以使用 select 和 map 来完成，但是 Ruby 2.7 引入了 filter_map，这是一个单步组合你在这里有什么。

有关filter_map的更多详细信息，请参阅“In Ruby, is there an Array method that combines 'select' and 'map'?”。

【讨论】：