当我使用许多非常短的方法阅读代码时,我发现通常需要比它应该更长的时间来理解所有东西是如何组合在一起的。您的示例代码中很好地说明了一些原因。让我们尝试将其分解为微小的方法:
def cart
if user_signed_in?
process_current_user
else
setup_cart
end
end
private
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
私有以外的所有方法。这就引入了另一个问题。我是否将所有私有方法放在最后——在这种情况下,我可能必须滚动过去几个不相关的方法才能找到它们——或者我是否在整个代码中交替使用公共和私有方法?(当然,如果我能记住哪个代码文件做了什么,这个问题可以通过保持较小的代码文件和使用 mixins 得到一定程度的缓解。)
还要考虑代码行数的变化。代码行越多,出错的机会就越多。(我故意犯了一个常见的错误来说明这一点。你发现了吗?)测试单个方法可能更容易,但我们现在需要测试许多单独的方法是否可以一起正常工作。
现在让我们将其与稍微调整的方法进行比较:
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
而不是将其分配给实例变量,并避免分配值:@user
cart
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