9
\$\begingroup\$

I'm currently learning Ruby and need help in editing code to be more idiomatic. I have the method below I could use some pointers on how to make more "Ruby" :-)

def print_cal(day_of_week, month_len)
  days = %w(Mon Tue Wed Thu Fri Sat Sun)
  days.each { |day| print "#{day} " }

  puts

  day_num = 1
  month_started = false
  while day_num <= month_len
    (1..7).each do |i|
      if  day_of_week > i && month_started == false
        print '    '
      else
        month_started = true
        print ' ' if day_num < 10
        print " #{day_num} " if day_num <= month_len
        day_num += 1
      end
    end
    puts
  end
end
\$\endgroup\$
1
  • \$\begingroup\$ Well done using the %w( ), nice indentation, I'm a little confused as to what you're trying to do with print ' '? \$\endgroup\$
    – 13aal
    Dec 22, 2015 at 12:00

3 Answers 3

4
\$\begingroup\$
  1. I would rather return array of strings than the whole text -- this gives ability to loop over or even reverse lines
  2. I would totally get rid of temp variables and their assignments
  3. I would use even another approach to interpolate number into string: "%3s" % date

And some other tricks:

def format_calendar offset, month_length
  [*1..month_length].unshift(*Array.new(offset)).
    each_slice(7).map{ |week|
      week.map{ |date| "%3s" % date }.join " "
    }.unshift "Mon Tue Wed Thu Fri Sat Sun"
end

puts format_calendar 2, 31

Or if you find unshift ugly, try this, but it does pointless interpolation for weekday titles:

def format_calendar offset, month_length
  [
    * %w{Mon Tue Wed Thu Fri Sat Sun},
    * Array.new(offset),
    * (1..month_length),
  ].each_slice(7).map{ |week|
    week.map{ |date| "%3s" % date }.join " "
  }
end

puts format_calendar 2, 31
\$\endgroup\$
2
  • \$\begingroup\$ I like the second solution. I'm not entirely sure how the splat operator works in this case though. Do we need it on Array.new(offset)? Or is it just there for "symmetry". Could the same be done without a splat? Splats seem a bit cryptic to use in a language like ruby . \$\endgroup\$
    – Leo Net
    Oct 2, 2014 at 17:48
  • \$\begingroup\$ @LeoNet, without splat the array would be nested -- [[a,b],[c,d],[e,f]], where Array.new is [c,d]. Instead of splat you could use concatenations but had to add () around all of it to apply the .each_slice method: ( %w{} + Array.new() + (1..).to_a ).each_slice \$\endgroup\$
    – Nakilon
    Oct 3, 2014 at 16:29
11
\$\begingroup\$

Here's my take, with explanations below:

def print_calendar(offset, month_length)
  puts "Mon Tue Wed Thu Fri Sat Sun"
  dates = [nil] * offset + (1..month_length).to_a
  dates.each_slice(7) do |week|
    puts week.map { |date| date.to_s.rjust(3) }.join(' ')
  end
end
  1. I'd call the first parameter offset rather than day_of_week. For one, day_of_week would seem to imply that you can use a Date object's wday as the offset, but wday's range is 0-6, with zero being Sunday, so that won't work. Also, with day_of_week I wonder if you mean a day's name, or its number? Oh, and which week are we talking about? The name offset is a lot more generic, but that might be a good thing here.
    I've also spelled out print_calendar and month_length entirely. While those names weren't too confusing, there's no reason to skip a few letters.

  2. Also note that the offset behaves slightly different from yours: In your method, the minimum offset is 1. That makes sense if you think "start on the 1st day", but it makes less sense if you think "offset everything by 1". So here, the minimum offset is zero.

  3. Array#join is a nice method to know when you have to print lists of stuff (in any language, not just Ruby). You could have done puts %w(Mon Tue Wed Thu Fri Sat Sun).join(' ') instead of your first 3 lines. But the end result would be no different than simply printing a hard-coded string, so that's what we're doing here instead.

  4. In Ruby, you rarely have to use while loops. Generally, it's easier to construct and manipulate arrays using the built-in methods in Array and those mixed-in by the Enumerable module. (Seriously, if you're learning Ruby, go read as much as you can from those two links; it's some the most useful stuff in Ruby, and you'll be using it a lot.)
    Here, we're constructing a dates array of nils that's offset items long by using Ruby's nifty ability to multiply an array by a number. Then we use + to concatenate that array with an array that's just the numbers 1..month_length.
    In other words, dates consists of zero or more "blanks" (the offset) followed by the date numbers of the month.

  5. each_slice should be fairly self-explanatory: Go through an array, X items at a time. In this case we go through it 7 items at a time (i.e. a week), which is what we need for each row/line of text.

  6. Each of these weeks are just arrays of nils and/or numbers, so we convert each of them, using map, to an array of strings. Each blank/number is first converted to a string, and then padded with spaces using rjust. Finally, this mapped array is joined and output.

And that's it, really.

Calling print_calendar(2, 31) will print the following:

Mon Tue Wed Thu Fri Sat Sun
          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

However, it'd be even more idiomatic to let the method simply return the text without printing it. That could be done like so:

def format_calendar(offset, month_length)
  lines = [ "Mon Tue Wed Thu Fri Sat Sun" ]
  dates = [nil] * offset + (1..month_length).to_a
  dates.each_slice(7) do |week|
    lines << week.map { |date| date.to_s.rjust(3) }.join(' ')
  end
  lines.join("\n")
end

After which you can do something like

puts format_calendar(2, 31)

to print it.

Edit: As Naklion's answer rightly points out, it'd be even nicer to return an array of strings (the individual lines of text). In this case, you can do so by simply omitting the .join("\n") on the last line of the method above. That'll give you more structured data to work with, instead of a text blob.
As for printing, you don't need to change anything, since puts automatically adds linebreaks when printing an array.

\$\endgroup\$
5
  • \$\begingroup\$ I would even rather return array of strings, than the whole text. puts won't break, but you'll get more possibilities. \$\endgroup\$
    – Nakilon
    Oct 2, 2014 at 7:55
  • \$\begingroup\$ @Nakilon Good point. I'd forgotten that puts still does the right thing with that. I'll add a bit to my answer \$\endgroup\$
    – Flambino
    Oct 2, 2014 at 8:01
  • \$\begingroup\$ Couldn't resist from other things, so I already posted it in mine. \$\endgroup\$
    – Nakilon
    Oct 2, 2014 at 8:26
  • \$\begingroup\$ @Nakilon Even better :) I'll still add a small note, and point people toward your answer (I'd actually written the edit 20 minutes ago - I just forgot to click "Save"... :P ) \$\endgroup\$
    – Flambino
    Oct 2, 2014 at 8:29
  • \$\begingroup\$ Thank you for your excellent explanations I have learned a lot!!! \$\endgroup\$
    – Leo Net
    Oct 7, 2014 at 19:26
3
\$\begingroup\$

For more idiomatic Ruby,

  • Use puts instead of print to output a newline.
  • To make that happen, use map and join.
  • Construct your weeks using Enumerable#each_slice.

In addition, you should use sprintf to ensure that each cell is formatted to the same width. That works for the headers, the blank padding, one-digit dates, and two-digit dates.

def print_cal(starting_day_of_week, month_len)
  days = %w(Mon Tue Wed Thu Fri Sat Sun)   
  leading_pad = [nil] * (starting_day_of_week - 1)
  dates = (1..month_len).to_a

  calendar = days + leading_pad + dates
  calendar.each_slice(7) do |week|
    puts week.map { |date| sprintf('%3s', date) }.join(' ')
  end
end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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