Rails Calendar Generation Method

Question

I have this helper method that generates an HTML calendar based on the current day:

module PageHelper
  def calendar(date)
    cal = "<table class='table-condensed table-bordered table-striped'><thead><tr>"
    days = %w(Sun Mon Tue Wed Thu Fri Sat)
    days.each do |day|
      cal += "<th>#{day}</th>"
    end
    cal+= '</thead></tr><tbody><tr>'
    #end of table head
    first_day = date.at_beginning_of_month.strftime('%w').to_i
    last_day = date.at_end_of_month.strftime('%d').to_i
    last_day_week = date.at_end_of_month.strftime('%w').to_i
    current_day = date.strftime("%d").to_i
    prev_month_day = (date - 1.month).at_end_of_month.strftime('%d').to_i
    # print first row
    first_day.times do |day|
      num = prev_month_day - first_day + day + 1
      cal += "<td class='text-muted small'>#{num}</td>"
    end
    (7-first_day).times do |day|
      if day == current_day -1
        cal += "<td class='btn-primary'><strong>#{day+1}</strong></td>"
      else
        cal += "<td>#{day+1}</td>"
      end
    end
    # print remaining rows
    cal += '<tr>'
    (7-first_day).upto(last_day-1).each_slice(7) do |slice|
      cal += '<tr>'
      slice.each do |day|
        if day == current_day -1
          cal += "<td class='btn-primary'><strong>#{day+1}</strong></day>"
        else
          cal += "<td>#{day+1}</td>"
        end
      end
      cal += '</tr>' unless slice.last == last_day -1
    end
    # get remaining number of cells
    rem_cells = 6 - last_day_week
    if rem_cells && rem_cells != 6
      rem_cells.times do |day|
        cal += "<td class='text-muted'>#{day+1}</td>"
      end
    end
    cal += '</tr></tbody></table><br/><br/>'
  end
end

The method is called like this:

<%=  calendar(Date.today).html_safe %>

Here is an example calendar:

However, I think that my code is not very DRY and the method is unnecessarily longer than what it should be.

Answer 1

I agree that it is too long. I find a good place to start is to try breaking it up into methods that are short and each do a single thing. While I'm doing this I usually see opportunities to make things DRY.

My code might look like:

content_tag(:table, ...) do 
  header_row + week_rows      
end

def week_rows
  num_rows = (first_day + last_day) % 7
  rows = num_rows.times.map { |i| week_row(i) }
  content_tag(:tbody) do rows.join('')
end

A couple of things I noticed:

• Instead of hard-coding html like "<table class='table-condensed table-bordered table-striped'><thead><tr>" use the Rails content_tag helper

• Instead of date.at_end_of_month.strftime('%w').to_i use the wday method. There is also a day method that you can use instead of %d.

• Have your method return a string that is already html_safe