一种方法是可以使用ternary operator重构它，但要以可读性为代价。

def cart
  if user_signed_in?
    @user = current_user
    @cart = @user.cart.present? ? @user.cart : false
  else
    cart_id = session[:cart_id]
    @cart = cart_id.present? ? Cart.find(cart_id.to_i) : false
  end
end

其次，如果你被迫写一个很长的方法，这意味着你的面向对象设计有问题。也许，你需要为额外的功能设计一个新的类，或者你需要通过写多个方法来分割同一个类中的功能，当这些方法组合在一起时，完成一个长方法的工作。
为什么写一个行太多的方法是不好的？
就像一篇大段大段的文章很难读一样，一个方法很长的程序也很难读，而且很难重用。你把代码分成越多的块，你的代码就越模块化，越容易重用，越容易理解。
如果我们要在里面做很多工作呢？
如果你要在里面做很多工作;这肯定意味着你设计类的方式不好，也许你需要设计一个新的类来处理这个功能，或者你需要把这个方法分成更小的块。
我怎样才能重构它并写出好的代码呢？
为此，我强烈推荐给你一本很棒的书，书名是：Refactoring，他在解释如何、何时以及为什么重构代码方面令人难以置信。

当我阅读包含许多非常短的方法的代码时，我发现要理解每一个方法是如何组合在一起的，通常需要花费比预期更长的时间。其中一些原因在示例代码中得到了很好的说明。让我们尝试将其分解为小方法：

def cart
  if user_signed_in?
    process_current_user
  else
    setup_cart
  end
end

专用

def process_current_user
  @user = current_user
  if @user.cart.present?
    assign_cart_when_user_present
  else
    assign_cart_when_user_not_present
  end
end

def assign_cart_when_user_present
  @cart = @user.cart
end

def assign_cart_when_user_not_present
  @cart = false
end

def setup_cart
  cart_id = session[:cart_id]
  if cart_id.present?
    assign_cart_when_id_present
  else
    assign_cart_when_id_present
  end
end

def assign_cart_when_id_present
  @cart = Cart.find(cart_id.to_i)
end

def assign_cart_when_id_not_present
  @cart = false
end

现在有几个大问题：

• 在阅读了cart方法之后，我不知道它是做什么的，部分原因是值被赋给了示例变量，而不是返回值给调用cart的方法。
• 我希望方法process_current_user和setup_cart的名称能告诉读者它们的作用。这些名称没有做到这一点。它们可能会得到改进，但它们是我能想到的最好的方法。关键是，设计出信息丰富的方法名称并不总是可能的。

我想把除了cart之外的所有方法都设置为私有方法，这又引入了另一个问题，我是把所有私有方法放在最后--在这种情况下，我可能需要滚动几个不相关的方法才能找到它们--还是在整个代码中交替使用公有方法和私有方法？（当然，如果我能记住哪个代码文件做什么，通过保持代码文件较小并使用mixin，这个问题可以得到一定程度的缓解。）
再考虑一下代码行数的变化。代码行数越多，出错的机会就越多。（我故意犯了一个常见的错误来说明这一点。你注意到了吗？）测试单个方法可能更容易，但我们现在需要测试许多单独的方法是否能正常工作。
现在，让我们将其与稍微调整的方法进行比较：

def cart
  if user_signed_in?
    @user = current_user
    @cart = case @user.cart.present?
            when true then @user.cart
            else           false
            end
  else
    cart_id = session[:cart_id]
    @cart = case cart_id.present?
            when true then Cart.find(cart_id.to_i)
            else           false
            end
  end
end

这让读者一目了然地知道正在发生的事情：

• 如果用户已登录，则将@user设置为current_user;以及
• @cart被分配一个值，该值取决于用户是否登录以及在每种情况下是否存在ID。

我不认为测试这种大小的方法比测试我之前将代码分解成更小的方法更困难，事实上，确保测试是全面的可能更容易。
我们也可以返回cart，而不是将其赋给示例变量，如果用户登录后只需要确定cart，则可以避免将值赋给@user：

def cart
  if user_signed_in?
    cart = current_user.cart        
    case cart.present?
    when true then cart
    else           false
    end
  else
    cart_id = session[:cart_id]
    case cart_id.present?
    when true then Cart.find(cart_id.to_i)
    else           false
    end
  end
end

你有很好的答案以上，但请允许我建议一个不同的方式来写相同的方法...：

# by the way, I would change the name of the method, as cart isn't a property of the controller.
def cart
    # Assume that `current_user` returns nil or false if a user isn't logged in.
    @user = current_user

    # The following || operator replaces the if-else statements.
    # If both fail, @cart will be nil or false.
    @cart = (@user && @user.cart) || ( session[:cart_id] && Cart.find(session[:cart_id]) )
end

正如你所看到的，有时if语句是浪费的，知道你的方法在失败时返回什么可以使代码更容易阅读和维护。
顺便说一句，为了理解||语句，请注意：

"anything" && 8 # => 8 # if the statements is true, the second value is returned
"anything" && nil && 5 # => nil # The statement stopped evaluating on `nil`.
# hence, if @user and @user.cart exist:
@user && @user.cart #=> @user.cart # ... Presto :-)

祝你好运!

• • 附言**

我会考虑在Cart类中为此编写一个方法（或者在您看来这是Controller逻辑吗？）：

# in cart.rb
class Cart
   def self.find_active user, cart_id
      return user.cart if user
      return self.find(cart_id) if cart_id
      false
   end
end

# in controller:

@cart = Cart.find_active( (@user = current_user), session[:cart_id] )

• 假设错误的数量与代码的长度成正比，那么最好是让代码更短。
• 即使代码的长度保持不变（或者可能会稍微变长），最好还是将一个长方法分解成短方法，因为对于人类来说，一次阅读一个短方法并找到它的bug比通读一个长方法更容易。