F677fa685a2cfe8aff31f161062db3d3

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?

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 !

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy

July 31, 2008, July 31, 2008 00:17, permalink

No rating. Login to rate!

see my post below...

Avatar

timmaah.myopenid.com

July 31, 2008, July 31, 2008 01:37, permalink

No rating. Login to rate!
1
times = times - events.collect { |e| e.datetime }
5071c5b861341c0dcfcf6ac86327701f

Tien Dung

July 31, 2008, July 31, 2008 02:05, permalink

No rating. Login to rate!

Shortest version :)

1
times -= events.map(&:datetime)
F677fa685a2cfe8aff31f161062db3d3

ttdavett.myopenid.com

July 31, 2008, July 31, 2008 04:27, permalink

No rating. Login to rate!

This does not work, the date values are different in these arrays and i want to only compare the times.

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy

July 31, 2008, July 31, 2008 04:31, permalink

1 rating. Login to rate!

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'")) }
F677fa685a2cfe8aff31f161062db3d3

ttdavett.myopenid.com

August 1, 2008, August 01, 2008 08:50, permalink

No rating. Login to rate!

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
507ad9765784506b94461181f4d31d9a

Laurel Fan

August 7, 2008, August 07, 2008 02:42, permalink

2 ratings. Login to rate!

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)) }

Your refactoring





Format Copy from initial code

or Cancel