問題描述
替代在此 Ruby 代碼中使用基於時間間隔分配標籤的巨型 if/else (Alternative to using a giant if/else in this Ruby code that assigns labels based on the time interval)
I have a Ruby script that I inherited, where its reading a csv file containing "TV programs" that have a start time, and end time in this format:
start_time = 20:00:00
end_time = 20:45:00
The goal is to assign each TV program a "time‑slot" (one of the following values) based on the start and end times:
23:00:00 ‑ 05:00:00 = Late Night = l
05:00:00 ‑ 09:00:00 = Morning = m
09:00:00 ‑ 17:00:00 = Day Time = d
17:00:00 ‑ 20:00:00 = Evening = e
20:00:00 ‑ 23:00:00 = Prime = p
Right now I have a giant if/else statement that is about 100 lines of Ruby Code:
if(start_time >= 50000 && start_time < 90000) #start time is between 5 and 9 am
if(end_time <= 90000)
@timeSlot = ["Morning"]
puts "timeSlot = [Morning]"
elsif(end_time <= 170000 && end_time > 90000)
@timeSlot = ["Morning", "Daytime"]
puts "timeSlot = [Morning, Daytime]"
elsif(end_time <= 200000 && end_time > 90000 && end_time > 170000)
@timeSlot =["Morning", "Daytime", "Evening"]
puts "timeSlot =[Morning, Daytime, Evening]"
elsif(end_time <= 230000 && end_time > 90000 && end_time > 170000 && end_time > 200000)
@timeSlot =["Morning", "Daytime", "Evening", "Prime"]
puts "timeSlot =[Morning, Daytime, Evening, Prime]"
else
@timeSlot =["Morning", "Daytime", "Evening", "Prime", "LateNight"]
puts "timeSlot =[Morning, Daytime, Evening, Prime, LateNight]"
end
elsif (start_time >= 90000 && start_time < 170000)
.........
........
end
Im trying to change the implementation so the code is easy to maintain and extend and read. My first try at this problem was to solve it visually using a matrix in excel as shown.
This is the problem displayed visually. Now the question is how to do this in code in an efficient way?
Any advice is welcome
參考解法
方法 1:
One more variant...
require 'time'
RANGES = [
["00:00:00", "Late Night"],
["05:00:00", "Morning"],
["09:00:00", "Day Time"],
["17:00:00", "Evening"],
["20:00:00", "Prime"],
["23:00:00", "Late Night"]]
NBR_PERIODS = RANGES.size
LAST_PERIOD = NBR_PERIODS ‑ 1
class TimesToList
def initialize
@ranges = []
RANGES.each { |r| @ranges << Time.strptime(r.first,"%H:%M:%S") }
end
def list_of_periods(start_time, end_time)
start_period = secs_to_period(Time.strptime(start_time, "%H:%M:%S"))
end_period = secs_to_period(Time.strptime(end_time, "%H:%M:%S"))
((start_time <= end_time) ? (start_period..end_period).to_a :
(start_period..LAST_PERIOD).to_a + (0..end_period).to_a).map {|p|
p == 0 ? LAST_PERIOD : p}.uniq.sort.map {|p| RANGES[p].last}
end
private
def secs_to_period(t) NBR_PERIODS.times {|i|
return i if i == LAST_PERIOD or t < @ranges[i+1]}
end
end
TimesToList.new.list_of_periods("23:48:00", "10:15:00")
# => ["Morning", "Day Time", "Late Night"]
I am going to assume the question "...do this in code in an efficient way?" is about how to come up with a more elegant solution to the problem over getting a better efficient runtime algorithm.
First off, I notice that your nested if statements contain redundant condition checks, e.g.
elsif(end_time <= 200000 && end_time > 90000 && end_time > 170000)
The only way this condition will be true is if end_time <= 200000 and end_time > 170000, and therefore it is not necessary to check the condition end_time > 90000. You only need to check your upper and lower bounds for each conditional statement for these statements.
Second, You could also greatly reduce the number of if statements, by intially pushing the start onto the array, and then the respective end times instead of hardcoding the values of the array for each and every condition. Take this code for instance
@timeSlot = []
# for each record r in csv file
if(start_time >= 50000 && start_time < 90000)
@timeSlot.push "Morning"
elsif (start_time >= 90000 && start_time < 170000)
@timeSlot.push "Day Time"
....
end
if(end_time <= 170000 && end_time > 90000)
@timeSlot.push "Daytime"
elsif ...
then use a function to remove any duplicates in the @timeSlot array. Now you will see though that you are dynamically generating the array instead of hardcoding all your combinations, which is typically not something programmers have time for.
Another thing you can do to make your code more maintainable over time is not use hardcoded literals. you should have constant variable for each significant time slice, e.g
TIME_5AM = 50000
TIME_9AM = 90000
...
then use those variables in the if statements instead. This will reduce typo bugs maybe 5000 accidently over 50000, etc.
Hope that is a helpful push in the right direction.
方法 3:
Although your business rules have few short comings mentioned by others, it is interesting. Here is how I approached it.
require 'time'
start_time = "20:00:00"
end_time = "20:45:00"
start_time = Time.strptime(start_time,"%H:%M:%S")
end_time = Time.strptime(end_time,"%H:%M:%S")
slot_times = { :late_night => {start: "23:00:00", end: "05:00:00", slot: "Late Night"},
:morning => {start: "05:00:00", end: "09:00:00", slot: "Morning"},
:day_time => {start: "09:00:00", end: "17:00:00", slot: "Day Time"},
:evening => {start: "17:00:00", end: "20:00:00", slot: "Evening"},
:prime => {start: "20:00:00", end: "23:00:00", slot: "Prime"} }
slot_times.each do |k,v|
x,y = Time.strptime(v[:start],"%H:%M:%S"), Time.strptime(v[:end],"%H:%M:%S")
puts v[:slot] if start_time.between?(x,y) || end_time.between?(x,y) #=> Evening, Prime
end
方法 4:
There are several ways to optimize the code base.
- Write numbers with underscores (ruby ignores them in numbers, but they enhance readability).
5_00_00equals 5 am. This way it is easier to separate hours from minutes. - You could formulate the conditions for each time slot in a
Proc, this gives the condition a readable name. - The slots array of a TV‑show is mutable. So we can add a matching time slow if the condition applies.
show.slots << 'Day Time' if <day time condition>. This way we do not need deeply netsedif's
It tried to apply the above steps in the following listing:
# we need some overhead to define valid data for this example
require 'ostruct'
require 'pp'
shows = [
OpenStruct.new(:start_time => 12_35_00, :end_time => 13_00_00, :slots => [], :name => 'weather stuff'),
OpenStruct.new(:start_time => 4_00_00, :end_time => 07_15_00, :slots => [], :name => 'morning show'),
OpenStruct.new(:start_time => 18_45_00, :end_time => 20_15_00, :slots => [], :name => 'vip news'),
OpenStruct.new(:start_time => 06_12_00, :end_time => 23_59_00, :slots => [], :name => 'home shopping')
]
# overhead done, the show can begin :)
def ranges_overlap?(a, b)
a.include?(b.begin) || b.include?(a.begin)
end
time_slots = {
:late_night => Proc.new {|s| ranges_overlap?(23_00_00..23_59_59, s.start_time..s.end_time) or ranges_overlap?(0_00_00..5_00_00, s.start_time..s.end_time) },
:morning => Proc.new {|s| ranges_overlap?(5_00_00..9_00_00, s.start_time..s.end_time) },
:day_time => Proc.new {|s| ranges_overlap?(9_00_00..17_00_00, s.start_time..s.end_time) },
:evening => Proc.new {|s| ranges_overlap?(17_00_00..20_00_00, s.start_time..s.end_time) },
:prime => Proc.new {|s| ranges_overlap?(20_00_00..23_00_00, s.start_time..s.end_time) }
}
shows.each do |show|
time_slots.each do |name, condition|
show.slots << name if condition.call(show)
end
end
pp shows
# [#<OpenStruct start_time=123500, end_time=130000, slots=[:day_time], name="weather stuff">,
# #<OpenStruct start_time=40000, end_time=29504, slots=[:late_night], name="morning show">,
# #<OpenStruct start_time=184500, end_time=201500, slots=[:evening, :prime], name="vip news">,
# #<OpenStruct start_time=25216, end_time=235900, slots=[:late_night, :morning, :day_time, :evening, :prime], name="home shopping">]
I borrowed the ranges_overlap? from this SO discussion.
方法 5:
class Time_Slot < Struct.new(:name, :begin, :end)
def overlap?(range)
range.include?(self.begin) || range.begin.between?(self.begin, self.end)
end
end
TIME_SLOTS = [
Time_Slot.new(:Late_night, 0, 5),
Time_Slot.new(:Morning, 5, 9),
Time_Slot.new(:Day_time, 9, 17),
Time_Slot.new(:Evening, 17, 20),
Time_Slot.new(:Prime, 20, 23),
Time_Slot.new(:Late_night, 23, 24)]
def calc_slots(start, stop)
range = start...stop
TIMESLOTS.select{|ts| ts.overlap?(range)}.map(&:name)
end
p calc_slots(1,20) #=>[:Late_night, :Morning, :Day_time, :Evening]
(by banditKing、Cary Swoveland、Gary Drocella、Bala、tessi、steenslag)