views:

63

answers:

1

I have an Event model with a finish_time field and a form checkbox called whenever. When whenever is checked I want to set finish_time to nil regardless of its value in parameters, when whenever is not checked I want to make sure that finish_time is a valid date before I save it in the database.


Currently I am achieving this using a whenever attr_accessor on Event:

params[:event][:finish_time] = nil if whenever = params[:event].delete(:whenever)
@event = Event.new(params[:event])
@event.whenever = whenever

And using conditional validations to check for finish_time if whenever is false

validates_presence_of :finish_time, :unless => @whenever


I am not happy with the duplication this creates. The only way that finish_time can be nil is if whenever is true, and if you change one you often have to change the other. If the Event is updated with a new finish_time then whenever will have to be changed to false as well.

Ideally I would like to move the validation to my controller. That way when doing direct model access whenever can be easily indicated by setting finish_time to null, but users posting to /events will get yelled at if they haven't indicated a choice. However I can't figure out a way to conditionally add or remove validations to Event from the controller or even if this is the best approach.

What is the best way to remove this duplication?

+1  A: 

Instead of trying to solve problem via validation, you might try to address it in "a Ruby way".

In addition to validation, you might want to hook into mutators of finish_time and whenever methods.

class Event < ActiveRecord::Base
  # Validate presence
  validates_presence_of :finish_time, :unless => @whenever

  def whenever=(yes)
    write_attribute(:whenever, yes)
    write_attribute(:finish_time, nil) if yes?
  end
  def finish_time=(time)
    write_attribute(:whenever, false)
    write_attribute(:finish_time, time)
  end
end

Then you can just try (however, I'm not sure whether it works):

@event = Event.new(params[:event])

Edit: However, this will make the resultant fields depend on the order assignemnts happen, so the results for the same parameter set may differ if the order of assignments changes. That's unlikely what you want. So you must tune your opertators the way you want.

And you also should understand that you won't be able to make your code DRY unless you make your requirements DRY.

Edit #2: I think you should consider dropping the requirement for finish_time to be nil if whenever is set to true. You can then modify accessors in the same way as described above.

Pavel Shved
Good point about keeping my requirements DRY. Maybe there is some Date value other than null that could indicate "don't care" instead of empty? I think the problem comes from making one value mean two things. I can't think of any way that doesn't resort to magic numbers.
maxhawkins
edited answer with reqs suggestion
Pavel Shved
That makes a lot of sense. Thanks!
maxhawkins