patternrubyrailsMinor
Checking for valid date range in Rails
Viewed 0 times
rangecheckingdaterailsforvalid
Problem
I am getting a date range from form inputs - the "from" month, day, and year, and the "to" month, day, and year, and then running a report on data for within that date range. I want to first check that the dates are valid and if not display an error indicating dates are not valid, then run the report only if valid. I want to
I am having trouble doing this in a clean way with repeating myself a bunch. Here's the long way. Please help refactor with assigning to variables but only if valid.
- make sure the dates themselves are valid, i.e., no blank inputs, and nothing like February 30th, then
- make sure the "from" date is before the "to" date.
I am having trouble doing this in a clean way with repeating myself a bunch. Here's the long way. Please help refactor with assigning to variables but only if valid.
unless Date.valid_date?(params[:from][:year].to_i,
params[:from][:month].to_i,
params[:from][:day].to_i) &&
Date.valid_date?(params[:to][:year].to_i,
params[:to][:month].to_i,
params[:to][:day].to_i)
redirect_to :back, alert: "Please provide valid date values." and return
end
unless Date.new(params[:from][:year].to_i,
params[:from][:month].to_i,
params[:from][:day].to_i) <
Date.new(params[:to][:year].to_i,
params[:to][:month].to_i,
params[:to][:day].to_i)
redirect_to :back, alert: "Please provide valid date values." and return
end
from_date = Date.new(params[:from][:year].to_i,
params[:from][:month].to_i,
params[:from][:day].to_i)
to_date = Date.new(params[:to][:year].to_i,
params[:to][:month].to_i,
params[:to][:day].to_i)
run_report(from_date, to_date)Solution
DRY - don't repeat yourself - basically means that you should extract repeated logic into methods.
In this case you're repeating
So you could for instance do this instead:
In this case you're repeating
- checking validity
- creating
Dateinstances (if they're valid, you should just create them, and then compare them; it's madness to create them for the comparison only to re-create them a few lines later)
- ... and for each of those, you're extracting 3 specific values from a params hash
So you could for instance do this instead:
# Returns a Date, or nil if the input isn't valid
def params_to_date(params = {})
args = %w(year month day).map { |k| params[k] }
Date.new(*args) if Date.valid_date?(*args)
end
# ...
def some_controller_action
from_date = params_to_date(params[:from])
to_date = params_to_date(params[:to])
if from_date && to_date && from_date < to_date
run_report(from_date, to_date)
else
redirect_to :back, alert: "Please provide valid date values."
end
endCode Snippets
# Returns a Date, or nil if the input isn't valid
def params_to_date(params = {})
args = %w(year month day).map { |k| params[k] }
Date.new(*args) if Date.valid_date?(*args)
end
# ...
def some_controller_action
from_date = params_to_date(params[:from])
to_date = params_to_date(params[:to])
if from_date && to_date && from_date < to_date
run_report(from_date, to_date)
else
redirect_to :back, alert: "Please provide valid date values."
end
endContext
StackExchange Code Review Q#110262, answer score: 6
Revisions (0)
No revisions yet.