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
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.
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With