Presenting a time as a string

Question

I have three methods that are really similar but do slightly different things. This doesn't feel very DRY to me, but I can't think of a way to reduce the duplication. Here are the methods, I think they're fairly self explanatory:

# Present a date as a string, optionally without a year
# Example output: "December 1, 2011"
def date_string(year=true)
 if date
 str = "%B %e"
 str += ", %Y" if year
 date.strftime(str).gsub(' ',' ')
 else
 ""
 end
end
# Present a time as a string, optionally with a meridian (am/pm)
# Example output: "1:30 PM"
def start_time_string(meridian=true)
 if start_time
 str = "%l:%M"
 str += " %p" if meridian
 start_time.strftime(str).lstrip
 else
 ""
 end
end
# Present a time as a string, optionally with a meridian (am/pm)
# Example Output: "2:00"
def end_time_string(meridian=true)
 if end_time
 str = "%l:%M"
 str += " %p" if meridian
 end_time.strftime(str).lstrip
 else
 ""
 end
end

Each method just presents a time as a string with certain options, and if the time object they're trying to present is nil, they return an empty string.

Any ideas how to DRY this up?

Answer 1

To complement @mjgpy3's answer, some notes:

• def date_string(year=true). That's idiomatic Python, but in Ruby the idiomatic is def date_string(year = true). Personally I don't like positional optional values, specially booleans; when you call them you have no idea what the argument stands for. You can use an options hash instead. Do you remember the old Rails obj.save(false)? wisely changed in Rails 3 to obj.save(:validate => false).

• How to build string with conditional values: This is the pattern I use:

  str = ["%B %e", *(", %Y" if year)].join
  

• the else returns an empty string, why? it's better if it returns nil.

• Check String#squish.

To sum it up, I'd write the first method like this:

def date_string(options = {})
 format = ["%B %e", *(", %Y" if options[:year])].join
 date.strftime(format).squish
end

Just as a side note: does it make sense to have all these methods in a model? (or a presenter, it doesn't matter). Think of an orthogonal approach, create helper methods available for everbody to use (and send them date object), this way you achieve real modularity.

• Andrew: The reason for returning an empty string is these methods get chained together in some places, so if one of them returns nil it actually needs to be an empty string so I can join it to another string without checking for nil. Great suggestions though, this is really helpful to consider. I think you're right about extracting it to be more general purpose formatting helpers, I'll move the code that direction. Thanks!
• Andrew: Also, string.squish is awesome, wow. Thanks for showing me that!
• tokland: Regarding the empty string, yeah, sometimes is handy to return an empty string. But usually empty strings are not that useful, you want to let the caller decide how to handle it: method_that_may_return_nil || default_value. With empty strings you still can do it, but it's more verbose: method_that_may_return_empty_string.presence || default_value

Answer 2

Well, I don't see much you can do with the first method (being as it has a different responsibility than the other two). You can just combine the latter two methods into one and make the time object a parameter, like such:

def time_string(time_obj, meridian=true)
 if time_obj
 str = "%l:%M"
 str += " %p" if meridian
 time_obj.strftime(str).lstrip
 else
 ""
 end
end

This should reduce complexity some, since both latter methods have essentially the same responsibility.