How can I make this massive Ruby if/elsif statement more compact and cleaner?

Question

The following if/elsif statement is clearly a behemoth. The purpose of it is to change the phrasing of some text based on if certain data has been filled in by the user. I feel like there's got to be a better way to do this without taking up 30+ lines of code, but I'm just not sure how since I'm trying to customize the text pretty significantly based on the data available.

if !birthdate.blank? && !location.blank? && !joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}.</p>"
elsif !birthdate.blank? && !location.blank? && !joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location} and is #{time_ago_in_words(birthdate)} old. #{sex} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>"
elsif birthdate.blank? && !location.blank? && !joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}.</p>"
elsif birthdate.blank? && !location.blank? && !joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>"
elsif birthdate.blank? && location.blank? && !joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}. #{sex} passed away on #{death.strftime("%B %e, %Y")}.</p>"
elsif birthdate.blank? && location.blank? && !joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>"
elsif !birthdate.blank? && location.blank? && !joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}.</p>"
elsif !birthdate.blank? && location.blank? && !joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} and is #{time_ago_in_words(birthdate)} old. #{sex} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>"
elsif !birthdate.blank? && !location.blank? && joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
elsif !birthdate.blank? && !location.blank? && joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location} and is #{time_ago_in_words(birthdate)} old. #{sex} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
elsif !birthdate.blank? && location.blank? && joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
elsif !birthdate.blank? && location.blank? && joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} and is #{time_ago_in_words(birthdate)} old. #{sex} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
elsif birthdate.blank? && !location.blank? && joined.blank? && !death.blank?
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
elsif birthdate.blank? && !location.blank? && joined.blank? && death.blank?
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
else
  "<p class='birthinfo'>#{name} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
end

Answer 1

You should be able to use deMorgan's theorem to rewrite the query so it is evaluated more efficiently.
You can also be factor out common subexpressions into outer if statements.
However, you will likely be sacrificing clarity of intent when you do so.
Nowadays it's probably better form to write a clear, if long if statement rather than a short incomprehensible one.

Answer 2

I would be tempted to write convenience methods that describe each of those states, if there's a simple descriptive term for them. It wouldn't result in less code, but it would result in more readable code. That code is nearly impossible to read.

Looking at it, though, I'm not sure what kind of descriptive convenience method names you could come up with. You may just have to deal with the ugliness given what you're trying to do.

Answer 3

One thing I can think of is instead of performing each query, perform all of the queries and assign the results of the boolean variables, then test the booleans. That doesn't reduce LOC, but it does reduce line length. Forget what I said earlier about switch statements, that was not useful in the least. What you might try doing is breaking each condition into several methods. The first thing checked in the given if statement would still be there, but that would call one of two methods for the next condition and so on and so forth. This will actually expand LOC (which I don't feel matters as much) but readability will be increased. It is not the optimal solution, but it will work.

Answer 4

I think that you want to make all of these conditions more readable, and eliminate the repetition that exists both in your logic checks and in your string creation.

The first thing that I notice is that you repeat this all over the place:

<p class='birthinfo'>#{name} was born in

I would try to factor these out into either functions that take arguments and return formatted text, or classes that evaluate into expressions.

You're also not taking advantage of nesting at all. Instead of having every condition check the value of four expressions, you should try something like this:

if birthdate.blank?
    # half of your expressions
else
    # other half of your expressions

It might not make your code more readable, but it's worth trying out. The last thing that I'd suggest is that you might be able to cleverly rearrange your text so that it is both easy to construct piecewise and still reads well to the end-user. Here's an example:

notice = "#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location}."

One code snippet to generate this could be:

notice = "#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")}"
if !location.is_blank? 
    notice += " in #{location}"
end
notice += "."

That's much easier to read that the analogous code that you have, which would check each condition separately and make a totally different string depending on the values of the boolean variables. The worst part about the logic that you have now is that if you add a fifth variable, you have to add a lot more special cases to your code. Building up your code into independent pieces would be much easier to read and maintain.

Answer 5

You could put the strings in an array then make an array index based on whether each of your variables are blank:

strings = [
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location} and is #{time_ago_in_words(birthdate)} old. #{sex} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} in #{location} and is #{time_ago_in_words(birthdate)} old. #{sex} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} and is #{time_ago_in_words(birthdate)} old. #{sex} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")}. #{sex} passed away on #{death.strftime("%B %e, %Y")} at the age of #{calculate_age(birthdate, death)}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>",
  "<p class='birthinfo'>#{name} was born on #{birthdate.strftime("%A, %B %e, %Y")} and is #{time_ago_in_words(birthdate)} old. #{sex} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>",
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}.</p>",
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>",
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} passed away on #{death.strftime("%B %e, %Y")}. #{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>",
  "<p class='birthinfo'>#{name} was born in #{location}. #{sex} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>",
  "<p class='birthinfo'>#{name} was a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{distance_of_time_in_words(joined,death)}. #{sex} passed away on #{death.strftime("%B %e, %Y")}.</p>",
  "<p class='birthinfo'>#{name} has been a member of #{link_to user.login, profile_path(user.permalink)}'s family for #{time_ago_in_words(joined)}.</p>",
  "", # This case was missing from your code. (Where all are blank except 'death'.)
  "<p class='birthinfo'>#{name} is a member of #{link_to user.login, profile_path(user.permalink)}'s family.</p>"
]

index = 0
index += 1 unless death.blank?
index += 2 unless joined.blank?
index += 4 unless location.blank?
index += 8 unless birthdate.blank?

return strings[index]

Answer 6

I'd break it down into parts. DRY. Only generate a given segment of text once. Use a StringIO to keep the string generation separable.

sio = StringIO.new("")
know_birthdate, know_location, did_join, has_died = [ birthdate, location, joined, death ].map { |s| !s.blank? }

print_death = lambda do 
  sio.print ". #{sex} passed away on #{death.strftime("%B %e, %Y")}"
end
show_birth = know_birthdate or know_location

sio.print "<p class='birthinfo'>#{name} "
if show_birth
  sio.print "was born" 
  sio.print " on #{birthdate.strftime("%A, %B %e, %Y")}" if know_birthdate
  sio.print " in #{location}" if know_location
  if has_died
    print_death[]
    sio.print " at the age of #{calculate_age(birthdate, death)}" if know_birthdate
  elsif know_birthdate 
    sio.print " and is #{time_ago_in_words(birthdate)} old"
  end
  sio.print ". #{sex} "
end
sio.print "#{(has_died ? "was" : did_join ? "has been" : "is")} a member of #{link_to user.login, profile_path(user.permalink)}'s family"
sio.print " for #{distance_of_time_in_words(joined,death)}" if did_join and has_died
print_death[] if has_died and not show_birth
sio.print ".</p>"

sio.to_s

This makes the logic much easier to follow, and makes it much easier to make changes.

Answer 7

Here's a snipped demonstrating how I would do it:

ret = []
ret << "<p class='birthinfo'>#{name}"
ret << "was born on #{birthdate.strftime("%A, %B %e, %Y")}" unless birthdate.blank?
ret << "in #{location}." unless location.blank?
ret << sex
ret << "passed away on #{death.strftime("%B %e, %Y")}" unless death.blank?
ret << "at the age of #{calculate_age(birthdate, death)}"
ret << "#{sex} was a member of #{link_to user.login, profile_path(user.permalink)}'s family"
ret << 'for #{distance_of_time_in_words(joined,death)}" unless joined.blank? || death.blank?
ret << '.</p>'

ret.join(' ')