在模型重构：什么错？

Question

我目前试图干涸初始冗长的代码：

def planting_dates_not_nil?
    !plant_out_week_min.blank? || !plant_out_week_max.blank? || !sow_out_week_min.blank? || !sow_out_week_max.blank?
  end

  def needs_planting?(week)
    if !plant_out_week_min.blank? && !plant_out_week_max.blank?
      (plant_out_week_min..plant_out_week_max).include? (week) 
    end
  end

  def needs_sowing?(week)
    if !sow_out_week_min.blank? && !sow_out_week_max.blank?
      (sow_out_week_min..sow_out_week_max).include? (week)
    end 
  end

  def needs_harvesting?(week)
    if !harvest_week_min.blank? && !harvest_week_max.blank?
      (harvest_week_min..harvest_week_max).include? (week) 
    end
  end

这是我的INTIAL尝试：

  def tasks_for_week(week,*task_names)
    task_names.each do |task_name|
      to_do_this_week = []
        unless read_attribute(task_name).nil?
          if (read_attribute("#{task_name}_week_min")..read_attribute("#{task_name}_week_max")).include? (week)
            to_do_this_week << task_name
          end
        end
      end
  end

然而，当我在控制台上运行该代码，如下所示：

p.tasks_for_week(Date.today.cweek, :plant_out, :sow_out])

我得到了意想不到的结果......即使植物并不需要种植的时候，我仍然得到这两个任务的名称数组返回（[：plant_out，：sow_out]

任何人都可以让我知道我是怎么想打扫一下，并有tasksforweek方法返回预期的结果？

TIA

Answer 1

您方法返回task_names.each的结果。 each总是返回它开始。所以，你需要真正的回报你的结果。

此外，你现在重新创建你to_do_this_week上循环的每一次迭代，这将擦拭干净阵列。

def tasks_for_week(week, *task_names)
  to_do_this_week = []
  task_names.each do |task_name|
    if some_condition
      to_do_this_week << task_name 
    end
  end
  to_do_this_week
end

或者这样的：

def tasks_for_week(week, *task_names)
  returning [] do |to_do_this_week|
    task_names.each do |task_name|
      if some_condition
        to_do_this_week << task_name 
      end
    end
  end
end

但我认为这可能是你最好最好的：

def tasks_for_week(week, *task_names)
  task_names.find_all do |task_name|
    some_condition
  end
end

这最后一个通过一个阵列使用find_all其中迭代，并且将返回填充的任何对象的新数组块返回一个真值。

最后，你条件逻辑是有点疯狂了。你可以使用活动记录字段[]存取以动态的方式。而且它通常是清晰的，而不是使用unless something.nil?的双重否定的正侧。如果这是创建范围共同使用，它可能是最好的农场出来的方法：

def week_range_for_task(task)
  self["#{task_name}_week_min"]..self["#{task_name}_week_max"]
end

...

self[task_name] && week_range_for_task(task_name).include?(week)

使得整个方法：

def tasks_for_week(week, *task_names)
  task_names.find_all do |task_name|
    self[task_name] && week_range_for_task(task_name).include?(week)
  end
end

Answer 2

一件事上述需要注意的是self[task_name]似乎从数据库中获取的原始数据，忽略你可以写任何自定义getter方法。

如果你想使用自定义的getter，或者如果您有任何方法，你想当作属性，你可以用它代替self.send(task_name) self[task_name]。

Answer 3

这是与在那里新的条件的方法稍作修改代码。

  def whole_range_exists?(method_name)
        self["#{method_name}_week_min"] && self["#{method_name}_week_max"]
      end

      def week_range_for_task(task_name)
        self["#{task_name}_week_min"]..self["#{task_name}_week_max"]
      end

      def tasks_for_week(week, *task_names)
        task_names.find_all do |task_name|
          whole_range_exists?(task_name) && week_range_for_task(task_name).include?(week)
        end
      end