On Thu, Feb 28, 2008 at 03:29:54AM +0900, Brian Adkins wrote:
> On Feb 27, 1:00 pm, Gregory Seidman <gsslist+r... / anthropohedron.net>
> wrote:
> > On Thu, Feb 28, 2008 at 02:49:54AM +0900, Brian Adkins wrote:
> > > On Feb 27, 11:40 am, Gregory Seidman <gsslist+r... / anthropohedron.net>
> > > wrote:
> > > > On Thu, Feb 28, 2008 at 01:17:22AM +0900, Shandy Nantz wrote:
> > > > > This is probably an easy question but I am trying to get at the number
> > > > > of days that are in a month. I have this calendar that I have built, the
> > > > > idea being that when a month turns from February to March, for example,
> > > > > the calendar should redisplay itself properly formated showing the new
> > > > > month and the correct number of days. I have it so that it starts
> > > > > counting the days on the right day of the week, but I have to know when
> > > > > to stop counting. Any ideas, Thanks,
> >
> > > > require 'date'
> >
> > > > def days_in_month(month, year)
> > > >   month = month.to_i
> > > >   year = year.to_i
> > > >   raise ArgumentError.new("invalid month") unless (1..12).to_a.include? month
> > > >   first = Date.parse sprintf("%04d%02d01", year, month)
> > > >   next_month = first + 32
> > > >   (last - last.mday).mday
> > > > end
> >
> > > > > -S
> > > > --Greg
> >
> > > might want to try running that before posting
> >
> > Ah, details. Change the last line of the method to:
> >
> > (next_month + next_month.mday).mday
> >
> > Anyhow, it's worth noting that ActiveSupport includes Time.days_in_month.
> >
> > --Greg
> 
> You still didn't run it, did you? <sigh>

Don't sigh at me. I wrote some code off the cuff and fired it off. I also
didn't include unit tests. Yes, it was buggy and inefficient, but it got
across the approach I was using.

> Some ideas you may want to consider:
> 1) it's probably reasonable to expect numeric month and day arguments,
> so you can skip the .to_i calls

Given that it was for clarity, I think it's valuable.

> 2) instead of creating a range, converting it to an array and calling
> include?, wouldn't it be better to just use a simple comparison such
> as "unless month > 0 && month < 13

Arguable. I prefer range inclusion to a pair of comparisons, but that's a
matter of taste. The to_a only matters if I hadn't performed a to_i on the
month argument previously.

> 3) sprintf'ing a date just to parse it is unnecessary & inefficient

True enough. Date.new (a.k.a. Date.civil) takes year, month, and day
arguments. For that matter, as pointed out elsewhere in this thread, a -1
for the day argument gives the last day of the month, making the rest of
the method moot.

> 4) it's still broken

Typo. The + should have been a - in the correction. The correct, if
unnecessary, method is:

require 'date'

def days_in_month(month, year)
  month = month.to_i
  year = year.to_i
  raise ArgumentError.new("invalid month") unless (1..12).include? month
  first = Date.civil(year, month, 1)
  next_month = first + 32
  (next_month - next_month.mday).mday
end

An even simpler method, taken from elsewhere in the thread:

require 'date'

def days_in_month(month, year)
  Date.civil(year, month, -1).mday
end

--Greg