如何在 Ruby 中重构长的 if/return 语句？

Question

我经常遇到一堆复杂的if语句，Ruby 的清理方法是什么？

（在这个服务对象示例中，一个 foo 有很多条。这是为了将一个 bar 转移到不同的 foo。）

class BarManager
  include FancyErrorLogger

  def self.transfer(bar, new_foo)

    # Is a move needed? Is this line superfluous and a premature optimisation?
    return true if bar.foo_id == new_foo.id

    # Checks that bar can be moved to new_foo. Many more elsifs in practice, needs refactoring. These examples demonstrate the potential complexity of each step, preventing the use of overly simplistic solutions such as seen here http://codereview.stackexchange.com/questions/14080/avoiding-a-lot-of-ifs-in-ruby
    if bar.dependency == :do_not_move_me or bar.some_condition == false
      bar.errors.add( :transfer, "This is the bar that can't be moved, it is on street corners moping and singing")
      return false
    elsif new_foo.want_more_bars == false
      bar.errors.add( :transfer, "\"We don't take kindly to your type, bar\" - #{new_foo.name}")
      return false
    elsif ((bar.baz.optional_external_nightmare_status != :unused) || (new_foo.bars.inject(false){|result, element| result = result || element.baz.optional_external_nightmare_status != :unused}))
      bar.errors.add( :transfer, "I have some baz news for you...")
      return false
    elsif bar.yet_another_failure_reason_there_are_many
      bar.errors.add( :transfer, "There are many ways for this to fail, this if statement is somewhat short")
      return false
    elsif bar.stubborn?
      bar.lure_with_carrot!
      if bar.munching?
        bar.errors.add ( :transfer, "eh, what's up doc" )
        return false
      elsif !bar.following?
        bar.errors.add ( :transfer, "Your carrot is too small and inadequate. No jokes please" )
        return false
      end
    end

    # We made it through the gauntlet, now for the transfer
    cache_old_foo_id = bar.foo_id # we might need this
    bar.foo_id = new_foo.id
    bar.save!

    # If we are using Rails counter caching:
    Foo.increment_counter(:bars_count, new_foo.id)
    Foo.decrement_counter(:bars_count, cache_old_foo_id) # we DID need it

    return true

  rescue Exception => e
    fancy_error_log e
    false
  end

end

Answer 1

第 1 步是从那条长if语句中删除重复项。if返回一个值（如果没有if/elsif匹配则返回 nil ），所以你可以这样做

error =
  if bar.dependency == :do_not_move_me or bar.some_condition == false
    "This is the bar that can't be moved, it is on street corners moping and singing"
  elsif new_foo.want_more_bars == false
    "\"We don't take kindly to your type, bar\" - #{new_foo.name}")
  elsif ((bar.baz.optional_external_nightmare_status != :unused) || (new_foo.bars.inject(false){|result, element| result = result || element.baz.optional_external_nightmare_status != :unused})
    "I have some baz news for you..."
  elsif bar.yet_another_failure_reason_there_are_many
    "There are many ways for this to fail, this if statement is somewhat short"
  elsif bar.stubborn?
    bar.lure_with_carrot!
    if bar.munching?
      "eh, what's up doc"
    elsif !bar.following?
      "Your carrot is too small and inadequate. No jokes please"
    end
  end
if error
  bar.errors.add :transfer, error
  return false
end

第 2 步是将条件提取到名称易于理解的方法中，这在任何情况下都是一个好主意。将每个条件中的所有内容都移动到其方法中，甚至是!运算符，并为每个方法提供相同的参数列表。我们马上就会明白为什么。

error =
  if bar.unmovable? new_foo
    "This is the bar that can't be moved, it is on street corners moping and singing"
  elsif bar.unwanted? new_foo
    "\"We don't take kindly to your type, bar\" - #{new_foo.name}")
  elsif bar.has_bad_news? new_foo
    "I have some baz news for you..."
  elsif bar.will_fail_for_yet_another_reason? new_foo
    "There are many ways for this to fail, this if statement is somewhat short"
  elsif bar.lure_with_carrot? new_foo # if you don't like the side effects, lure it before the if
    "eh, what's up doc"
  elsif bar.uninterested_in_carrot? new_foo
    "Your carrot is too small and inadequate. No jokes please"
  end
# use error as above

第 3 步是再次删除重复项：

checks = {
  unmovable?: "This is the bar that can't be moved, it is on street corners moping and singing",
  unwanted?: "\"We don't take kindly to your type, bar\" - #{new_foo.name}"),
  has_bad_news?: "I have some baz news for you...",
  will_fail_for_yet_another_reason?: "There are many ways for this to fail, this if statement is somewhat short",
  lure_with_carrot?: "eh, what's up doc",
  uninterested_in_carrot?: "Your carrot is too small and inadequate. No jokes please"
}
method_name, error = checks.find { |method_name, _| bar.send :method_name, new_foo }
# use error as above