3
\$\begingroup\$

I got a Reservation model that has reservation_start and reservation_end. Then I have the usual date validations (end date cannot be lesser than start date and so on).

The problem is with these two guys:

  def reservation_start_available
    reservations = Reservation.where(["transport_id =?", transport_id])
    date_ranges = reservations.map { |e| e.reservation_start..e.reservation_end }

    date_ranges.each do |range|
      if range.include? reservation_start
        errors.add(:reservation_start, "Start date not available")
      end
    end
  end

  def reservation_end_available
    reservations = Reservation.where(["transport_id =?", transport_id])
    date_ranges = reservations.map { |e| e.reservation_start..e.reservation_end }

    date_ranges.each do |range|
      if range.include? reservation_end
        errors.add(:reservation_end, "End date not available")
      end
    end
  end

This is not even close to the DRY we love so much. How can I pass a parameter to the methods like this?

\$\endgroup\$
5
\$\begingroup\$

Introducing custom validators!

Assuming you validate your dates with this "fresh" syntax:

validates :reservation_start,        # Note the comma!
  availability: true                 # ...and some other validations
validates :reservation_end,
  availability: {name: "End date"}   # same here

Define this validator separately, a common place is app/validators, in this case that file would be named availability_validator.rb

class AvailabilityValidator < ActiveModel::EachValidator

  # `each` stands for 'each attribute with a validation'
  def validate_each(record, attribute, value)
    # Args: a model instance, a symbol of attribute and a value it has
    # You also get a hash in `options`
    # If you specified validation as `availability: true`, you wouldn't get it

    reservations = Reservation.where(["transport_id =?", record.transport_id])
    date_ranges = reservations.map { |e| e.reservation_start..e.reservation_end }

    date_ranges.each do |range|
      if range.include? value
        errors.add(attribute, "#{options[:name] || 'Date'} not available")
      end
    end

  end
end

Sure this example needs some refinement, but it demonstrates most tools you have at hand.

  • The first validation gets no options, because there's true specified, we avoid errors by using a default name with ||. Not the best practice (merging with a separate hash with defaults is), but when you don't have many parameters and places, that will do.
  • Extraction of transport_id into options could come in handy if validation like this is broadly used.
  • "Availability" is a bit too common term, you might need to rename this class.
  • Stuff like error messages and attribute names is better be put into a locale file and fetched using I18n.t 'some.key.name'.
  • You're doing too much on Ruby side. When armed with a relational database, you can do stuff like this:

    Reservation.where(transport_id: transport_id). # period tells there are further calls
      where("reservation_start <= :date AND reservation_end >= :date", date: value).
      exists? # returns true if there's a reservation with our value in range
    

    If a lower bound is smaller and the upper bound is bigger than our balue, then our value is in the range of that entry. SQL can handle it.

  • There is a possible error: the validation (even yours) will false-accept the range if the submitted range encloses one existing range completely (start and end are both outside bounds of any other range). I'm not sure if that's what you want, so be advised. This can be fixed by an extra validation, I'm leaving this up to you.

\$\endgroup\$
1
\$\begingroup\$

How about combining them?

def reservation_available
  reservations = Reservation.where(transport_id: transport_id)
  date_ranges = reservations.map { |e| e.reservation_start..e.reservation_end }

  date_ranges.each do |range|
    %i(reservation_start reservation_end).each do |start_or_end|
      next unless range.send(:include?, start_or_end)
      errors.add(start_or_end, "#{text_of(start_or_end)} date not available")
    end
  end
end

private

def text_of(start_or_end)
  start_or_end == :reservation_start ? 'Start' : 'End'
end
\$\endgroup\$
2
  • \$\begingroup\$ That sounds nice. What is the reason of using range.send(:include?, start_or_end) instead of range.include? reservation_start \$\endgroup\$ – Cássio Godinho Dec 2 '14 at 23:28
  • \$\begingroup\$ range.send(:include?, start_or_end) stands in for doing range.include? reservation_start and then range.include? reservation_end \$\endgroup\$ – Dan Kohn Dec 3 '14 at 2:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.