Please help me with this Ruby code
I am new to Ruby and I know that I am not using the simple and powerful methods available within it. I made my code work but it has to be simple not this huge (especially I feel I am very bad at loop variables)
i = 0
j = 0
loop_count = ((to_date-from_date)/(60*60*24)).to_i#diff b/w two dates in days
loop_count.times do
48.times do
event = Icalendar::Event.new
status = get_availability_on_date_and_hour(@templates, @availabilities, from_date+j.days, i).downcase
if(status != 'unavailable')
#Initialize start and end dates in DateTime's civil format
bias_date_time = DateTime.civil(from_date.y开发者_JAVA百科ear, from_date.month, from_date.day)
event.dtstart = bias_date_time + j.day + (i/2).to_i.hour + (i%2*30).to_i.minutes
event.dtend = event.dtstart + 30.minutes
event.summary = status.upcase
cal.add_event(event)
end
i += 1
end
i = 0
j += 1
end
Nothing particularly Ruby specific, but in general keep your methods short..
I would extract the loop contents into a separate method. The method name is used to describe what is going on.
I would also extract the calculation into a separate method to describe what it is you are trying to do and keep the complicated bits (the parts most likely to need changing) separate..
Something like :
0.upto(((to_date-from_date)/(60*60*24)).to_i) do |j|
0.upto(48) do |i|
status = get_availability_on_date_and_hour(@templates, @availabilities, from_date+j.days, i).downcase
add_event(from_date, to_date, i, j, status) if status != 'unavailable'
end
end
def add_event(from_date, to_date, i, j, status)
event = Icalendar::Event.new
event.dtstart = whatever_i_am_trying_to_calculate(from_date, i, j)
event.dtend = event.dtstart + 30.minutes
event.summary = status.upcase
cal.add_event(event)
end
def whatever_i_am_trying_to_calculate(from_date, i, j)
bias_date_time = DateTime.civil(from_date.year, from_date.month, from_date.day)
bias_date_time + j.day + (i/2).to_i.hour + (i%2*30).to_i.minutes
end
Also give your variables i and j useful names. i and j don't mean very much.
You can get rid of the initializing and incrementing of the counter variables i
and j
like this:
0.upto(((to_date-from_date)/(60*60*24)).to_i) do |j|
0.upto(48) do |i|
event = Icalendar::Event.new
status = get_availability_on_date_and_hour(@templates, @availabilities, from_date+j.days, i).downcase
if(status != 'unavailable')
#Initialize start and end dates in DateTime's civil format
bias_date_time = DateTime.civil(from_date.year, from_date.month, from_date.day)
event.dtstart = bias_date_time + j.day + (i/2).to_i.hour + (i%2*30).to_i.minutes
event.dtend = event.dtstart + 30.minutes
event.summary = status.upcase
cal.add_event(event)
end
end
end
This code is 6 lines shorter than yours, can't do anything about the code within the loops, because I don't really understand what you're doing there.
Update: An Alternative that works too:
((to_date-from_date)/(60*60*24)).to_i.times do |j|
48.times do |i|
event = Icalendar::Event.new
status = get_availability_on_date_and_hour(@templates, @availabilities, from_date+j.days, i).downcase
if(status != 'unavailable')
#Initialize start and end dates in DateTime's civil format
bias_date_time = DateTime.civil(from_date.year, from_date.month, from_date.day)
event.dtstart = bias_date_time + j.day + (i/2).to_i.hour + (i%2*30).to_i.minutes
event.dtend = event.dtstart + 30.minutes
event.summary = status.upcase
cal.add_event(event)
end
end
end
We can get rid of the j
variable and avoid addition of j.days
everywhere and use date
from_date.to_i.step(to_date.to_i, 24*60*60) do |date|
0.upto(48) do |half_hour|
each_date = Time.at(date)
event = Icalendar::Event.new
status = get_availability_on_date_and_hour(@templates, @availabilities, each_date, half_hour).downcase
if(status != 'unavailable')
#Initialize start and end dates in DateTime's civil format
bias_date_time = DateTime.civil(each_date.year, each_date.month, each_date.day)
event.dtstart = bias_date_time + (i/2).to_i.hour + (i%2*30).to_i.minutes
event.dtend = event.dtstart + 30.minutes
event.summary = status.upcase
cal.add_event(event)
end
end
end
精彩评论