832ed6ace46d61032151f4e1864c057f

Actually I really don't know how to refactor it into a better way.

Thanks for your help.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
WEEKDAYS = %w(mon tue wed thu fri sat sun)

dates = [
  {:day => WEEKDAYS[0], :time => '9:00 - 20:00'},
  {:day => WEEKDAYS[1], :time => '9:00 - 20:00'},
  {:day => WEEKDAYS[2], :time => '9:00 - 20:00'},
  {:day => WEEKDAYS[3], :time => '8:00 - 19:00'},
  {:day => WEEKDAYS[4], :time => '9:00 - 20:00'},
  {:day => WEEKDAYS[5], :time => '8:00 - 19:00'},
  {:day => WEEKDAYS[6], :time => 'closed'}
]



def build_opening_hours dates
  ret = []

  dates.each do |day|
    d = ret.detect {|v| day[:time] == v[:time] }
    if d.nil?
      ret.push({:days => [day[:day]], :time => day[:time]})
    else
      d[:days] << day[:day]
    end
  end

  data = []

  ret.each do |v|
    previous = nil
    v[:days].each do |day|
      index = WEEKDAYS.index(day)
      day_text = "opening_hours_#{day}"
      if previous.nil?
        data.push([day_text, v[:time]])
      else
        if ((index - 1) == previous)
          data.last[0] = data.last[0].gsub(/ - .*$/, '')
          data.last[0] += ' - ' + day_text
        else
          data.last[0] += ', ' + day_text
        end
      end
      previous = index
    end
  end

  ret = []

  data.each do |d|
    ret.push(d.join(': '))
  end

  ret
end

p build_opening_hours(dates)

# [
# "opening_hours_mon - opening_hours_wed, opening_hours_fri: 9:00 - 20:00",
# "opening_hours_thu, opening_hours_sat: 8:00 - 19:00",
# "opening_hours_sun: closed"
# ]

Refactorings

No refactoring yet !

A8d3f35baafdaea851914b17dae9e1fc

Adam

October 8, 2008, October 08, 2008 19:59, permalink

1 rating. Login to rate!
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
class DayRange < String
  def initialize(days, prefix = nil)
    @prefix = prefix
    super ranges(days).join(", ")
  end
  
  protected
    def ranges(days)
      group_by_concurrent_days(days).map do |range|
        result  = [ "#{@prefix}#{WEEKDAYS[range.first]}" ]
        result << "#{@prefix}#{WEEKDAYS[range.last]}" unless range.size == 1
        result.join(" - ")
      end
    end
  
    def group_by_concurrent_days(days)
      days.inject([]) do |range,day|
        if range.empty? || day != range.last.last.succ
          range << [ day ]
        else
          range.last << day
          range
        end
      end
    end
end

class GroupedHours < Array
  def initialize(dates)
    super dates_to_grouped_array(dates)
  end
  
  protected  
    def dates_to_grouped_array(dates)
      group_days_by_time(dates).map do |time,days|
        "#{DayRange.new(days, "opening_hours_")}: #{time}"
      end
    end
  
    def group_days_by_time(dates)
      dates.inject({}) do |times,attributes|
        (times[attributes[:time]] ||= []) << WEEKDAYS.index(attributes[:day])
        times
      end
    end
end

# >> GroupedHours.new(dates)
# =>["opening_hours_thu, opening_hours_sat: 8:00 - 19:00",
# "opening_hours_mon - opening_hours_wed, opening_hours_fri: 9:00 - 20:00",
# "opening_hours_sun: closed"]
23132e1aa8457e11b243a43b578d51dc

Simo Niemelä

October 15, 2008, October 15, 2008 17:00, permalink

No rating. Login to rate!
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
dates = {
  :mon => '9:00 - 20:00',
  :tue => '9:00 - 20:00',
  :wed => '10:00 - 21:00',
  :thu => '9:00 - 20:00',
  :fri => '8:00 - 21:00',
  :sat => 'closed',
  :sun => 'closed'
}

class Hash
  def group
    result = {}
    self.each do |k, v|
      next if result.has_value?(v)
      keys = []
      self.each { |key, value| keys << key if value == v and !keys.include?(key) }
      result[keys] = v
    end
    result.rehash
  end
end

class Array
  def separator(prefix, last_prefix)
    result = ''
    self.each_with_index do |k, i|
      last = ((i + 1) == (self.length - 1))
      sep = last ? last_prefix : prefix
      result += k
      result += sep unless i + 1 == self.length
    end
    result
  end
end

def build_opening_hours(dates)
  result = []
  dates.group.each do |keys, value|
    result << keys.map {|k| "opening_hours_#{k.to_s}" }.separator(' - ', ', ') + ": #{value}"
  end
  result
end


p build_opening_hours(dates)

# => ["opening_hours_fri: 8:00 - 21:00", 
# "opening_hours_sat, opening_hours_sun: closed", 
# "opening_hours_wed: 10:00 - 21:00", 
# "opening_hours_tue - opening_hours_mon, opening_hours_thu: 9:00 - 20:00"]
832ed6ace46d61032151f4e1864c057f

Dmitry Polushkin

October 16, 2008, October 16, 2008 06:18, permalink

No rating. Login to rate!

@tmpr: You have an error in your output. Must be:
"opening_hours_mon - opening_hours_tue, opening_hours_thu: 9:00 - 20:00
Also must be sorted in a better way, not: sun, sat, fri, mon, tue... but mon, tue, wed... etc.
Also is very strange:
"opening_hours_sat, opening_hours_sun: closed", I think it must be "opening_hours_sat - opening_hours_sun: closed", by the previous example (opening_hours_tue - opening_hours_mon, opening_hours_thu: 9:00 - 20:00) logic?

I like code of the previous one, by Adam.

832ed6ace46d61032151f4e1864c057f

Dmitry Polushkin

October 16, 2008, October 16, 2008 06:20, permalink

No rating. Login to rate!

I have some idea how to improve it, but don't have free time to implement. Will write my refactoring in about a one-two weeks.

Your refactoring





Format Copy from initial code

or Cancel