重构模型：出了什么问题？

我目前正在干涸初始冗长的代码：重构模型：出了什么问题？

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]

任何人都可以让我知道如何清理这个问题，并有taskweweek方法返回预期的结果吗？

TIA

来源

2009-02-14 The Pied Pipes

你的方法返回的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

来源

2009-02-14 23:51:54

再次感谢Squeegy花时间重新构造这么充分。真的帮助我了解基础知识。我假设我可以使用select来代替find_all。 – 2009-02-15 14:45:42

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

如果您想使用自定义获取者，或者如果您有任何方法要处理为属性，则可以使用self.send(task_name)而不是self[task_name]。

来源

2009-02-15 01:53:54 Luke

这是稍微修改的代码，其中有一个新的条件方法。

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

来源

2009-02-15 15:05:31

重构模型：出了什么问题？

回答

相关问题