patternrubyMinor
Leap year program
Viewed 0 times
leapprogramyear
Problem
I'm switching to Ruby from Java and so, for exercise purposes, I wrote this verbose leap year Ruby program. The comments express my concerns about Ruby best practices. I'd be glad if some Ruby experienced programmer could dispel my doubts. Every hint such as "in Ruby you can achieve this thing better this way" is truly welcomed.
First of all, this is a possible execution:
Please note the verb conjugation and that every line has a comma at the end, except the last one that has a period.
`# I just felt bad about calling Time.now.year every time just to know the present year,
# is this simple memoization a good solution?
def was_is_or_will_be(year)
@present ||= Time.now.year
return 'is' if year == @present
return 'was' if year @present
end
# first of all, let's ask for the years interval
puts 'Pick a starting year:'
starting = gets.chomp.to_i
puts 'Now pick an ending year:'
# Is this the best "repeat..until" pattern for ruby?
# Because it doesn't seems very clear to me.
# There is no way to write somethin more explicit like a "do..while"?
while true
ending = gets.chomp.to_i
if ending > starting
break
end
puts "The ending year must be greater than the starting year,\n please pick a valid ending year:"
end
# This piece of code seems 'ruby-way' to me... isn't it?
leap_years = []
(starting..ending).each do |year|
if year%4 == 0 && (year%100 != 0 || year%400 == 0)
leap_years 0
puts "Specifically:"
# Is there a way to achieve the same function (a comma at the end of every line but a period at the end of the last one)
# Without an '
First of all, this is a possible execution:
Pick a starting year:
2000
Now pick an ending year:
2024
From year 2000 to year 2024 there are 7 leap years.
Specifically:
The year 2000 was a leap year,
The year 2004 was a leap year,
The year 2008 was a leap year,
The year 2012 is a leap year,
The year 2016 will be a leap year,
The year 2020 will be a leap year,
The year 2024 will be a leap year.
Please note the verb conjugation and that every line has a comma at the end, except the last one that has a period.
`# I just felt bad about calling Time.now.year every time just to know the present year,
# is this simple memoization a good solution?
def was_is_or_will_be(year)
@present ||= Time.now.year
return 'is' if year == @present
return 'was' if year @present
end
# first of all, let's ask for the years interval
puts 'Pick a starting year:'
starting = gets.chomp.to_i
puts 'Now pick an ending year:'
# Is this the best "repeat..until" pattern for ruby?
# Because it doesn't seems very clear to me.
# There is no way to write somethin more explicit like a "do..while"?
while true
ending = gets.chomp.to_i
if ending > starting
break
end
puts "The ending year must be greater than the starting year,\n please pick a valid ending year:"
end
# This piece of code seems 'ruby-way' to me... isn't it?
leap_years = []
(starting..ending).each do |year|
if year%4 == 0 && (year%100 != 0 || year%400 == 0)
leap_years 0
puts "Specifically:"
# Is there a way to achieve the same function (a comma at the end of every line but a period at the end of the last one)
# Without an '
Solution
Nice effort, :) here are a few comments.
unless you are calling this as part of another class,
do not use @member syntax. What you really want is a global.
Shorter name :), plus it is better to capture it in case statement rather
than abuse return :).
Now, what you asked for is possible, for example,
However, I dont recommend it.
Use heredocs if you have more than one line to output.
Modularize a little bit. get_years is obviously a specific functionality.
It is rubyish (and smalltalkish) to use select, inject, collect etc rather than
explicit loops where possible.
Smaller width lines if you can :). It helps, and it looks good.
Also make use of case in these cases.
Choose to exit early rather than another indentation level
unless you are calling this as part of another class,
do not use @member syntax. What you really want is a global.
$present = Time.now.yearShorter name :), plus it is better to capture it in case statement rather
than abuse return :).
def tense(year)
return case
when year $present ; 'will be'
else 'is'
end
end
def get_years()
# first of all, let's ask for the years interval
puts 'Pick a starting year:'
starting = gets.chomp.to_i
puts 'Now pick an ending year:'Now, what you asked for is possible, for example,
#begin
# ending = gets.chomp.to_i
#end while ending <= startingHowever, I dont recommend it.
while true
ending = gets.chomp.to_i
break if ending > startingUse heredocs if you have more than one line to output.
puts <<-EOF
The ending year must be greater than the starting year,
please pick a valid ending year:
EOF
end
return [starting,ending]
endModularize a little bit. get_years is obviously a specific functionality.
starting,ending = get_years()It is rubyish (and smalltalkish) to use select, inject, collect etc rather than
explicit loops where possible.
leap_years = (starting..ending).select do |year|
year%4 == 0 && (year%100 != 0 || year%400 == 0)
endSmaller width lines if you can :). It helps, and it looks good.
Also make use of case in these cases.
puts "From year #{starting} to year #{ending} there " + case leap_years.count
when 0 ; "are no leap years."
when 1 ; "is 1 leap year."
else "are #{leap_years.count} leap years."
endChoose to exit early rather than another indentation level
exit 0 unless leap_years.count > 0
puts "Specifically:"
puts leap_years.map{|leap_year|
"The year #{leap_year} #{tense(leap_year)} a leap year"
}.join(",\n") + "."Code Snippets
$present = Time.now.yeardef tense(year)
return case
when year < $present ; 'was'
when year > $present ; 'will be'
else 'is'
end
end
def get_years()
# first of all, let's ask for the years interval
puts 'Pick a starting year:'
starting = gets.chomp.to_i
puts 'Now pick an ending year:'#begin
# ending = gets.chomp.to_i
#end while ending <= startingwhile true
ending = gets.chomp.to_i
break if ending > startingputs <<-EOF
The ending year must be greater than the starting year,
please pick a valid ending year:
EOF
end
return [starting,ending]
endContext
StackExchange Code Review Q#12136, answer score: 4
Revisions (0)
No revisions yet.