3

I want to try get a date from params into a date format, and if it can't then i would like to then assign it to a date from a year today.

This is what i tried.

valid_until = params[:valid_until].try(:to_date) || Date.today.next_year

The try method is cool because if the :valid_until date is nil it will just return nil. What i found though, is that if someone has an invalid date like "4790224374" then it will return a ArgumentError as invalid date. the :valid_until date will still be run against to_date.

I guess having this with a rescue seems like the only answer, just wondering if there is a smarter way of trying to cater for nils and invalid date errors, before setting it to the default next year.

EDIT:

You can read up about Try here.

You can read up about to_date here

legendary_rob
  • 12,792
  • 11
  • 56
  • 102
  • You could put a `rescue` in place of `||` and perhaps drop the `try`? That would be fairly tidy, but I guess the downside is that all exceptions would be caught. – Sam Starling Sep 30 '14 at 15:20
  • Yeah what i was thinking. its a bit of a tricky one. this could become messy very quickly, i have spend the last little while at looking at a few ideas, but all seem to add a few lines. – legendary_rob Sep 30 '14 at 15:22
  • 1
    when you aren't using ruby core, could you please include the libraries you are using to assist those who would like to help :) – Jeff Price Sep 30 '14 at 15:27
  • Sorry true story i edited for those looking for them :) – legendary_rob Sep 30 '14 at 15:31
  • 1
    true story i knew where to find them, but not everyone knows rails :) – Jeff Price Sep 30 '14 at 15:33

2 Answers2

2

You are misusing Object.try. This method is meant to fail silently if a method does not exist on an object. In this case, the method is there (so it is called) and that method fails.

This is not meant to replace a try/rescue block. A possible implementation is below.

def expiration_date(a_string)
  Date.parse(a_string)
rescue
  Date.today.next_year
end

valid_until = expiration_date(params[:valid_until])
Jeff Price
  • 3,229
  • 22
  • 24
1

Unless it's some kind of code golf, I wouldn't try to squeeze as much logic in one liner as possible. Besides, you probably want to have your controllers lean as possible. Why not to use some sane OO gut feel that tells you: "if I can't figure out the logic here, maybe I should extract it to separate method or a class"? F.i.

# app/services/expirer.rb
class AccountExpirer
  def self.expiration_date(user_input)
    return Date.today.next_year unless user_input.present?
    begin
      Date.parse(user_input)
    rescue ArgumentError
      Date.today.next_year
    end
  end
end

# some controller
valid_until = AccountExpirer.expiration_date(params[:valid_until])

But if you need to tell user that he has entered invalid data, I wouldn't stop here. You can extend your class with ActiveModel::Model (http://api.rubyonrails.org/classes/ActiveModel/Model.html), which will allow you to write proper validations, and use it in your forms (just like AR model).

Ernest
  • 8,701
  • 5
  • 40
  • 51
  • Ernest, I am curious as to your method and mine. Is there any reason you chose to only catch ArgumentErrors in your rescue and to gate against a nil (which would produce a TypeError) instead of just rescuing everything? – Jeff Price Sep 30 '14 at 16:02
  • @JeffPrice I wouldn't begin/rescue at all if I am honest. But you didn't give any information, about do you enforce date format, or any other expectations, so my first thought was to use Date.parse, and catch the exception it throws (Date.parse on "invalid" date will throw ArgumenError, instead of, f.i. Date:InvalidDateError - this is because this method is not ment to be avalidator). My point was different, i.e. that kind of logic should not stay in a controller. – Ernest Sep 30 '14 at 16:12
  • This controller handels uploads, and doesnt have a model to run any validations on it. But you are absolutely correct. it is bad practice to have captured data validated on the controller level, extracting that behavior into a service is also a nice idea. thank you! – legendary_rob Sep 30 '14 at 16:43
  • @TheLegend You can create a "model" (not connected to database at all), by including ActiveModel into any plain class. – Ernest Sep 30 '14 at 18:52