Question

please tell me whether these two methods, merge into one. And without repetitions?

  def parse_data_if_not_null
    unless self.date_string.blank?
      begin
        self.ends_at = DateTime.strptime(self.date_string, '%m/%d/%Y %H:%M').utc
      rescue ArgumentError
        errors.add(:date_string, _("Wrong date format, example: MM/DD/YYYY HH/MM"))
      end
    end
  end

  def validate_less_today
    begin
      if (DateTime.strptime(self.date_string, '%m/%d/%Y %H:%M') < DateTime.now)
        errors.add(:date_string, "must be current or future date")
      end
    rescue ArgumentError
      errors.add(:date_string, _("Wrong date format, example: MM/DD/YYYY HH/MM"))
    end
  end
Was it helpful?

Solution

What do you think about this method?

  def merged_method
    unless self.date_string.blank?
      begin
        self.ends_at = DateTime.strptime(self.date_string, '%m/%d/%Y %H:%M').utc
        if self.ends_at < DateTime.now
           errors.add(:date_string, "must be current or future date")
        end
      end
      rescue ArgumentError
        errors.add(:date_string, _("Wrong date format, example: MM/DD/YYYY HH/MM"))
      end
    end
  end

OTHER TIPS

The two methods are used differently. I think the best way to refactor this is to add a method that parses the string.

def parse_data_if_not_null
  return if date_string.blank?

  date = parse_date(date_string)
  return unless date

  self.ends_at = date.utc
end

def validate_less_today
  date = parse_date(date_string)
  return if !date || date < DateTime.now

  errors.add(:date_string, "must be current or future date")
end

def parse_date(string)
  DateTime.strptime(self.date_string, '%m/%d/%Y %H:%M')

rescue ArgumentError
  errors.add(:date_string, _("Wrong date format, example: MM/DD/YYYY HH/MM"))
  nil
end
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top