Рефакторинг в модели: что случилось?

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, который возвращает ожидаемые результаты?

ТИА

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) 

Making весь метод:

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], похоже, получает необработанные данные из базы данных, игнорируя любые пользовательские методы получения, которые вы можете записать.

Если вы хотите использовать пользовательские геттеры или у вас есть какие-либо методы, которые вы хотели бы рассматривать как атрибуты, вы можете использовать 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