I have a method that returns the year and month between a set of dates.
d = (from..to).map {|d| [ d.year, d.month ] }.uniq
I can iterate over each element like so:
d.each do |elm|
#For Year
puts elm[0]
#For Month Number
puts elm[1]
end
How can I change the method so that I can iterate as follows to enhance the readability of the code and in general make it easier as well?
elm.month_number
elm.year
I think you're looking for array auto-unpacking as described here: How to iterate over an array of arrays
d = (from..to).map {|d| [ d.year, d.month ] }.uniq
d.each do |year, month|
# For Year
puts year
#For Month Number
puts month
end
elm.month and this way uses simply month instead? If that's the case, I'd say you're technically right, but is that really a significant difference? Is there any reason to prefer elm.month to month?
Use a class or a structure. e.g.
YearMonth = Struct.new(:year, :month)
d = (from..to).map{|d| YearMonth.new(d.year, d.month)}.uniq
although, this is not an optimal way of doing it. Consider this:
d = (from.year .. to.year).flat_map { |year|
from_month = year == from.year ? from.month : 1
to_month = year == to.year ? to.month : 12
(from_month..to_month).map { |month| YearMonth.new(year, month) }
}
This avoids creating a huge array of dates, and another array that results from a map, and goes straight for months (even if it is not as compact).
What you are doing is redundant. Keep the date elements as is (I assume they are Date objects).
d = (from..to).to_a.uniq_by{|elm| [elm.year, elm.month]}
class Date
alias month_number month
end
d.each do |elm|
elm.year
...
elm.month_number
...
end
from and to as Date objects, I get undefined method uniq for Range. With some googling it looks like Range#map (like the OP used) is the only way to go ... Range#to_a returns an empty set. Am I missing something?to_a.(Date.new(2015,11,1)..Date.new(2015,5,1)).to_a returns [] ...?2015,5,1 to 2015,11,1You can do something like this too (create a CustomDate class with year and month attributes):
class CustomDate
attr_accessor :year, :month
def initialize(year, month)
self.year = year
self.month = month
end
end
(from..to).map { |d| CustomDate.new(d.year, d.month) }.uniq.each do |e|
puts "year: #{e.year}"
puts "month: #{e.month}"
end
There's some great discussion in the other answers, but it seems silly to iterate over all 365 days in each year just to get 12 date objects when it's not at all necessary.
Instead of iterating from from to to, calculate an integer number of months from each and then iterate over that range. Take a look:
from = Date.civil(2013, 7, 1)
to = Date.civil(2015, 11, 23)
from_mos = 12 * from.year + from.month - 1
to_mos = 12 * to.year + to.month - 1
dates = (from_mos..to_mos).map do |mos|
year, month = mos.divmod(12)
Date.civil(year, month + 1, 1)
end
Now dates is an Enumerable yielding a Date for the first day of each month, i.e. 2013-07-01, 2013-08-01, … 2015-11-01, which you can use as below;
dates.each do |date|
puts "Year #{date.year}, month #{date.month}"
end
# => Year 2013, month 7
# Year 2013, month 8
# ...
# Year 2015, month 10
# Year 2015, month 11
P.S. If you want to go to the trouble of defining a class or Struct, I recommend making it work directly with the Range API:
YearMonth = Struct.new(:year, :month) do
def succ
next_year, next_month = months.succ.divmod(12)
self.class.new(next_year, next_month + 1)
end
def <=>(other)
months <=> other.months
end
protected
def months
12 * year + month - 1
end
end
Now you can just do this:
from = YearMonth.new(2013, 7)
to = YearMonth.new(2015, 11)
(from..to).each do |ym|
puts "Year #{ym.year}, month #{ym.month}"
end
