1
\$\begingroup\$

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:

Calendar

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

\$\endgroup\$
1
\$\begingroup\$

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

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.