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
3 Answers 3
- I would rather return array of strings than the whole text -- this gives ability to loop over or even reverse lines
- I would totally get rid of temp variables and their assignments
- 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
-
\$\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– Leo Net2014年10月02日 17:48:36 +00:00Commented Oct 2, 2014 at 17:48
-
\$\begingroup\$ @LeoNet, without splat the array would be nested --
[[a,b],[c,d],[e,f]]
, whereArray.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– Nakilon2014年10月03日 16:29:04 +00:00Commented Oct 3, 2014 at 16:29
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
I'd call the first parameter
offset
rather thanday_of_week
. For one,day_of_week
would seem to imply that you can use aDate
object'swday
as the offset, butwday
's range is 0-6, with zero being Sunday, so that won't work. Also, withday_of_week
I wonder if you mean a day's name, or its number? Oh, and which week are we talking about? The nameoffset
is a lot more generic, but that might be a good thing here.
I've also spelled outprint_calendar
andmonth_length
entirely. While those names weren't too confusing, there's no reason to skip a few letters.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.
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 doneputs %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.In Ruby, you rarely have to use
while
loops. Generally, it's easier to construct and manipulate arrays using the built-in methods inArray
and those mixed-in by theEnumerable
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 adates
array ofnil
s that'soffset
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.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.Each of these
week
s are just arrays ofnil
s and/or numbers, so we convert each of them, usingmap
, to an array of strings. Each blank/number is first converted to a string, and then padded with spaces usingrjust
. 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.
-
\$\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– Nakilon2014年10月02日 07:55:58 +00:00Commented 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– Flambino2014年10月02日 08:01:18 +00:00Commented Oct 2, 2014 at 8:01 -
\$\begingroup\$ Couldn't resist from other things, so I already posted it in mine. \$\endgroup\$Nakilon– Nakilon2014年10月02日 08:26:08 +00:00Commented 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– Flambino2014年10月02日 08:29:52 +00:00Commented Oct 2, 2014 at 8:29
-
\$\begingroup\$ Thank you for your excellent explanations I have learned a lot!!! \$\endgroup\$Leo Net– Leo Net2014年10月07日 19:26:41 +00:00Commented Oct 7, 2014 at 19:26
For more idiomatic Ruby,
- Use
puts
instead ofprint
to output a newline. - To make that happen, use
map
andjoin
. - 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
%w( )
, nice indentation, I'm a little confused as to what you're trying to do withprint ' '
? \$\endgroup\$