1 2 3 4 5 6 7 8
times.each do |time| events.each do |event| if time.strftime("%I:%M %p") == event.datetime.strftime("%I:%M %p") times -= Array.[](time) break end end end
Refactorings
No refactoring yet !
timmaah.myopenid.com
July 31, 2008, July 31, 2008 01:37, permalink
1
times = times - events.collect { |e| e.datetime }
Tien Dung
July 31, 2008, July 31, 2008 02:05, permalink
Shortest version :)
1
times -= events.map(&:datetime)
ttdavett.myopenid.com
July 31, 2008, July 31, 2008 04:27, permalink
This does not work, the date values are different in these arrays and i want to only compare the times.
Jeremy
July 31, 2008, July 31, 2008 04:31, permalink
I posted this refactoring earlier... not sure why it hasn't shown up. It should be faster because each event time is formatted only once.
1 2
event_times = events.collect { |event| event.datetime.strftime("%I:%M %p") } times = times.reject { |time| event_times.include?(time.strftime("%I:%M %p'")) }
ttdavett.myopenid.com
August 1, 2008, August 01, 2008 08:50, permalink
This code doesnt seem to work. even with reject!, not sure why, but formatting the event time only once is a good idea. here is the revised code, faster i believe but not any shorter.
1 2 3 4 5 6 7 8 9 10
event_times = events.collect { |event| event.date.strftime("%I:%M %p") } times.each do |time| event_times.each do |event| if time.strftime("%I:%M %p") == event times -= Array.[](time) break end end end
Laurel Fan
August 7, 2008, August 07, 2008 02:42, permalink
I think Jeremy's code pretty much works except for a stray single quote. That might be avoided by using a constant instead of a hardcoded string :)
1 2 3
TIME_OF_DAY_FORMAT = "%I:%M %p" event_times = events.collect { |event| event.datetime.strftime(TIME_OF_DAY_FORMAT) } times = times.reject { |time| event_times.include?(time.strftime(TIME_OF_DAY_FORMAT)) }
times is an array of datetimes and events is a hash of objects in which .datetime is also a datetime. The goal of this code is to remove the times that are in the form "Hour:minute am/pm" from the times array that are present in the event hash objects. It seems like there may be a better way to do this. I tried using the select method with something like: times.select { |x| events.include_timestamp?(x)} where include_timestamp? is an added Array method. I dont think this code can get any faster, but maybe prettier?