patternrubyrailsMinor

Nasty Age Printing Method

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

printingnastyagemethod

Problem

I have this ugly age printing method. Can you do better?

def age(birth_date)
  today = Date.today
  years = today.year - birth_date.year
  months = today.month - birth_date.month
  (months -1) if today.day  1
    age =  "#{months} months"
  elsif years > 0 && months > 0
    age = years == 1 ? "1 year, #{months} months" : "#{years} years, #{months} months"
  elsif years > 0 && months < 0
    months = months + 12
    years = years - 1
    age = "#{months} months" if years == 0
    age = "1 year, #{months} months" if years == 1
    age = "#{years} years, #{months} months" if years == 1
  end
  age
end

Solution

Some notes:

(months -1) if today.day

There is a age = in every branch of the conditional, and finally age is returned. That's unnecessary and not idiomatic, in Ruby conditionals are expressions, so we don't need to create a variable.




- 
About this:

age = "#{months} months" if years == 0
age = "1 year, #{months} months" if years == 1


It's preferable not to use in-line conditionals when you are dealing with assignments or expressions (it's ok if you are performing side-effects, i.e.

fail("error") if x

I'd write:

def age(birth_date)
  today = Time.zone.today
  total_months = (today.year*12 + today.month) - (birth_date.year*12 + birth_date.month)
  years, months = total_months.divmod(12)
  strings = [[years, "year"], [months, "month"]].map do |value, unit|
    value > 0 ? [value, unit.pluralize(value)].join(" ") : nil
  end
  strings.compact.join(", ")
end

Note that, to keep it simple, this code (as yours) ignores the day of the month, which gives a slightly wrong answer.

Code Snippets

age = "#{months} months" if years == 0
age = "1 year, #{months} months" if years == 1

age = if years == 0
  "#{months} months"
elsif years == 1
  #{months} months" if years == 1
...
end

def age(birth_date)
  today = Time.zone.today
  total_months = (today.year*12 + today.month) - (birth_date.year*12 + birth_date.month)
  years, months = total_months.divmod(12)
  strings = [[years, "year"], [months, "month"]].map do |value, unit|
    value > 0 ? [value, unit.pluralize(value)].join(" ") : nil
  end
  strings.compact.join(", ")
end

Context

StackExchange Code Review Q#25003, answer score: 4

Revisions (0)

No revisions yet.