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

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

вот моя первая попытка:

  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

Следует отметить, что self[task_name] получает необработанные данные из базы данных, игнорируя любые пользовательские методы получения, которые вы можете написать.

Если вы хотите использовать пользовательские методы получения или если у вас есть какие-либо методы, которые вы хотите рассматривать как атрибуты, вы можете использовать self.send(task_name) вместо self[task_name].

Answer 2

Ваш метод возвращает результат 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 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