Get integers for all months between two dates

Rails
#1

I have the following method which should work in Rails, but I am not
sure if this is the best implementation. It looks a little clumsy, given
all the good helpers in rails. Am I missing a much better way to do
this?

def get_months_and_years(st,ed)
m = [st.month]
d = st.beginning_of_month
while d <= ed.beginning_of_month
m <<
d = d.next_month
end
return m
end

Thanks,

Tim

#2

On 9 August 2010 18:52, Tim B. [email protected] wrote:

 m << [date.month, date.year]
 d = d.next_month

end
return m
end

No doubt there is a slicker way, but I suggest that maybe the thing
that is a little clumsy is code that needs this functionality in the
first place ( I mean building the array of numbers ). Can you not
just keep the start and end dates and determine the month and year
values at the time you need them (if you really need them at all that
is).

Colin

#3

Colin,

Good point, but I am not sure of another way. I need to display
calendars that show all events in a group. I’ll look at the bigger
picture again.

Best,

tim

Colin L. wrote:

On 9 August 2010 18:52, Tim B. [email protected] wrote:

� � �m <<
� � �d = d.next_month
� �end
� �return m
end

No doubt there is a slicker way, but I suggest that maybe the thing
that is a little clumsy is code that needs this functionality in the
first place ( I mean building the array of numbers ). Can you not
just keep the start and end dates and determine the month and year
values at the time you need them (if you really need them at all that
is).

Colin

#4

I would do it this way:

def get_months_and_year(st, ed)
m = []
if ed >= st
years = ed.year - st.year
if years == 0
(st.month…ed.month).each{|month| m << [month, st.year] }
elsif years > 0
(st.month…12).each{|month| m << [month, st.year] }
(st.year+1…ed.year-1).each{|year| (1…12).each{|month| m <<
[month,
year] } }
(1…ed.month).each{|month| m << [month, ed.year] }
end
end
return m
end

Just for not processing date objects many times. This code would be
refactored.

Daniel Alejandro Gaytán Valencia
http://survey.richapplabs.com/index.php?sid=62661&lang=en

2010/8/9 Tim B. [email protected]

#5

On 9 August 2010 18:52, Tim B. [email protected] wrote:

I have the following method which should work in Rails, but I am not
sure if this is the best implementation. It looks a little clumsy, given
all the good helpers in rails. Am I missing a much better way to do
this?

As a couple of people have said, looking at this in isolation won’t
allow us to easily give suggestions of better solutions, but you could
certainly tidy that method a little.

Firstly, (personal preference time!) your variable names aren’t
helping your code be “self documenting”. Even in such a small method,
it’s frustrating to try to remember what all those single-character
variables are supposed to be doing. If they’re named to refer to what
they do, they will help other people (or you when you come back to it
in a couple of months!) understand what the intention behind their
existence is.

Every time you loop round, you calculate “st.beginning_of_month” - it
would be a bit more efficient to store the value for use in the
comparison each time. You never use “st” again, so I’d just overwrite
it.

The first element you populate the array with only has “st.month”, all
the others have the year too - not sure if this is a typo or intended?
Another potential typo is in the array push; you refer to a “date”
variable which I can’t see set anywhere. I assume this is actually
your “d” variable?

So, if I were writing your method to be clearer, it might look like
this:

def get_months_and_years(start_date, end_date)
end_date = end_date.beginning_of_month
return_value = [start_date.month]

working_date = start_date.beginning_of_month
while working_date <= end_date
  return_value << [working_date.month, working_date.year]
  working_date = working_date.next_month
end
return return_value

end

But…
My inclination would be to be a bit more “Ruby”… assuming you want
an array of arrays of month and year for every month between two
dates:

def get_months_and_years(start_date, end_date)
# it would be worth checking that the parameters are both valid
dates/times
(start_date…end_date).collect { date|
}.uniq
end

In that method I’m just taking the month and year for every date in
the range, and then returning unique values from the collection (so
one for each month/year combination. I’ve not got any time to look
into it, but I wonder if the Range.collect can step through
month-at-a-time rather than day-at-a-time…

I don’t know if it’s any quicker or much slower, but it’s certainly
more concise, and (to my eye) a bit clearer in its intention, and
looks a lot less like PHP :slight_smile:

HTH
Michael

#6

On 10 August 2010 19:42, Tim B. [email protected] wrote:

Colin,

Good point, but I am not sure of another way. I need to display
calendars that show all events in a group. I’ll look at the bigger
picture again.

Not sure exactly what you are doing of course, but could you write
view helper code that takes the start and end dates and generates the
appropriate view stuff?

Colin