Как я могу сделать этот массивный оператор Ruby if / elsif более компактным и чистым?

Question

Следующее выражение if / elsif явно бегемот. Цель этого - изменить формулировку некоторого текста в зависимости от того, были ли определенные данные заполнены пользователем. Я чувствую, что должен быть лучший способ сделать это, не занимая более 30 строк кода, но я просто не уверен, как, поскольку я пытаюсь довольно сильно настроить текст на основе доступных данных.

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

Я думаю, что вы хотите сделать все эти условия более читабельными и исключить повторение, которое существует как в ваших логических проверках, так и при создании строки.

Первое, что я замечаю, это то, что вы повторяете это повсюду:

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

Я бы попытался разделить их либо на функции, которые принимают аргументы и возвращают форматированный текст, либо на классы, которые оцениваются в выражения.

Вы также вообще не пользуетесь вложенностью. Вместо того, чтобы каждое условие проверяло значение четырех выражений, вы должны попробовать что-то вроде этого:

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

Это может не сделать ваш код более читабельным, но стоит попробовать. Последнее, что я хотел бы предложить, - это то, что вы могли бы хитро перестроить свой текст так, чтобы его было легко создавать по кусочкам и при этом хорошо читать конечному пользователю. Вот пример:

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

Один фрагмент кода для генерации может быть:

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

Гораздо проще прочитать тот аналогичный код, который у вас есть, который проверяет каждое условие в отдельности и создает совершенно другую строку в зависимости от значений логических переменных. Хуже всего то, что у вас есть в логике: если вы добавляете пятую переменную, вам нужно добавить гораздо больше особых случаев в ваш код. Сборка вашего кода в независимые части будет намного легче читать и поддерживать.

Answer 2

Я бы разбил его на части. DRY . Генерируйте данный сегмент текста только один раз. Используйте StringIO, чтобы сохранить генерацию строки отделимой.

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

Это значительно упрощает логику и позволяет вносить изменения.

Answer 3

Вы можете поместить строки в массив, а затем создать индекс массива на основе того, пуста ли каждая из ваших переменных:

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 4

Вот отрывок, демонстрирующий, как я бы это сделал:

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(' ')

Answer 5

Одна вещь, о которой я могу подумать, это вместо того, чтобы выполнять каждый запрос, выполнять все запросы и присваивать результаты логическим переменным, а затем проверять логические значения. Это не уменьшает LOC, но уменьшает длину строки. Забудьте то, что я говорил ранее о выражениях switch, это было бесполезно. То, что вы можете попробовать сделать, это разбить каждое условие на несколько методов. Первое, что проверялось в данном условии if, все еще было бы там, но это вызвало бы один из двух методов для следующего условия и так далее, и так далее. Это на самом деле расширит LOC (что, я думаю, не имеет большого значения), но удобочитаемость будет увеличена. Это не оптимальное решение, но оно будет работать.

Answer 6

Я хотел бы написать удобные методы, которые описывают каждое из этих состояний, если для них есть простой описательный термин. Это не приведет к меньшему количеству кода, но приведет к более читаемому коду. Этот код почти невозможно прочитать.

Однако, глядя на это, я не уверен, с какими именами описательных вспомогательных методов вы могли бы придумать. Возможно, вам просто придется иметь дело с безобразием, учитывая то, что вы пытаетесь сделать.

Answer 7

Вы должны иметь возможность использовать теорему Деморгана , чтобы переписать запрос, чтобы он оценивался более эффективно.
Вы также можете выделять общие подвыражения во внешние операторы if.
Тем не менее, вы, вероятно, будете жертвовать ясностью намерений, когда будете это делать.
В настоящее время, вероятно, лучше написать ясное, хотя и длинное выражение, чем краткое непонятное.