Rails - 有没有更好的方法来写这个？寻找重构建议

Question

在我有限的知识中，如果不将志愿者作为参数传递给状态方法，我不知道如何编写此代码。这通常不会成为问题，除非我需要所有不规则的志愿者。然后我必须遍历 Volunteers.all 并根据 status 方法评估每个。我觉得这里的开销太大了。

Class Volunteer < ActiveRecord::Base

  has_many :reports

  def status(volunteer)
    case
    when volunteer.reports.nought.where(:report_month => Report_range.months(6)).count > 5
        return 'inactive'
    when volunteer.reports.nought.where(:report_month => Report_range.months(6)).count >= 3
        return 'irregular'
    else
        return 'active'
    end
  end
end


class Report < ActiveRecord::Base
  belongs_to :volunteer
  scope :nought, -> {where("hours IS NULL") || where("hours: 0")}
end

module Report_range
  def self.months(range)
   Volunteer.first.reports.order("report_month desc").limit(range).pluck(:report_month)
  end
end

提前感谢您帮助菜鸟！

Answer 1

我会这样做：

class Volunteer < ActiveRecord::Base

  has_many :reports

  def status
    if number_of_recent_nought_reports > 5
      'inactice'
    elsif number_of_recent_nought_reports >= 3
      'irregular'
    else
      'active'
    end
  end

  private

  def number_of_recent_nought_reports
    # You could move the where call to scope in your `Report` class.
    @recent_nought_reports ||= reports.nought.where(:report_month => Report_range.months(6)).count
  end
end

Answer 2

def status(volunteer)
  case
  when volunteer.reports.nought.where(:report_month => Report_range.months(6)).count > 5
    return 'inactive'
  when volunteer.reports.nought.where(:report_month => Report_range.months(6)).count >= 3
    return 'irregular'
  else
    return 'active'
  end
end

您不需要传递志愿者，因为您正在调用已经可以直接访问报告的志愿者实例的方法状态。于是上面就变成了：

def status
  case
  when reports.nought.where(:report_month => Report_range.months(6)).count > 5
    return 'inactive'
  when reports.nought.where(:report_month => Report_range.months(6)).count >= 3
    return 'irregular'
  else
    return 'active'
  end
end

此外，您正在运行两次计数查询，这不是最佳的。我建议：

def status
  number_of_reports = reports.nought.where(:report_month => Report_range.months(6)).count
  case
  when number_of_reports > 5
    return 'inactive'
  when  number_of_reports >= 3
    return 'irregular'
  else
    return 'active'
  end
end

最后，案例返回匹配值，因此您不需要返回。

def status
  number_of_reports = reports.nought.where(:report_month => Report_range.months(6)).count
  case
  when number_of_reports > 5
    'inactive'
  when  number_of_reports >= 3
    'irregular'
  else
    'active'
  end
end

还有更多，但这只是一个开始。

Answer 3

您也可以将其指定为使用@Magnuss 的代码作为基点

class Volunteer < ActiveRecord::Base

  has_many :reports

  STATUSES = {(0..2) => 'active', (3..5) => 'irregular'}

  def status
    STATUSES.find(->{['inactive']}){|range,_| range === number_of_recent_nought_reports}.pop 
  end

  private

   def number_of_recent_nought_reports
     # You could move the where call to scope in your `Report` class.
     @recent_nought_reports ||= reports.nought.last_n_months(6).count
  end
end

class Report < ActiveRecord::Base
  belongs_to :volunteer
  scope :nought, -> {where("hours IS NULL") || where("hours: 0")}
  scope :last_n_months, ->(months){ where(:report_month => Report_range.months(months)) }
end

在我看来，这允许通过重新定义STATUSES 您认为合适的方式来添加更多状态的更大灵活性。

这样做是找到number_of_recent_nought_reports 在STATUSES 中定义的范围内的第一个结果，如果它落在['inactive'] 的任何范围之外，则返回然后pop 只取出地位。

我还将 report_month 范围移动到 Report 为 last_n_months。

Answer 4

你可以写：

def status(volunteer)
  case volunteer.reports
                .nought
                .where(:report_month => Report_range.months(6))
                .count
  when (6..Float::INFINITY) then 'inactive'
  when (3..5)               then 'irregular'
  else                           'active'
end