1

这是一段代码，我想知道是否应该对其进行重构以使其更符合 Clean Code 实践。

这是一个负责退还客户订单的类。

class RefundServiceInvoker {

    private $_orders;

    public function refundOrder() {   
        $this->getOrdersFromDB(); //This function gets all orders from DB and sets $_orders
        foreach ($this->_orders as $order) { 
            try {
                $order->refund(); //Some lines may throw an exception when refunded due to some business logic (ex. the order was already shipped)
                $this->updateOrderStatus('refunded')            
            } catch (Exception $e) {                   
                $this->logError($e);
                $this->sendMailToAdmin();
            }
        }
    }
}

当然，这段代码比我的原始代码高度简化。

我的主要问题是如果$order->refund();抛出异常，它将被捕获并记录到数据库，然后发送邮件。但是，如果$this->logError($e);它本身抛出异常怎么办？或者如果邮件服务器关闭并抛出异常怎么办？

如果数据库自己宕机并$this->getOrdersFromDB();抛出异常怎么办？

我的第一个解决方案是将所有内容包装在一个大文件中try{}catch{}：

public function refundOrder() {   
            try {              
            $this->getOrdersFromDB(); //This function gets all orders from DB and sets $_orders
            foreach ($this->_orders as $order) { 

                    $order->refund(); //Some lines may throw an exception when refunded due to some business logic (ex. the order was already shipped)
                    $this->updateOrderStatus('refunded')            
                } catch (Exception $e) {                   
                    $this->logError($e);
                    $this->sendMailToAdmin();
                }
            }
        }

但这意味着如果一个订单失败，那么所有订单都会失败！我应该try{}catch{}为整个功能放置 2 个，为每个订单放置另一个吗？但是在这种情况下，catch 中的函数也可能会抛出一个不会被捕获的异常。

笔记：

该应用程序是使用 Zend 框架 1.11.11 构建的。

提前致谢。

4

3 回答 3

3

没有灵丹妙药可以解决这些问题。如果一个函数可以抛出并且你关心它，你必须包裹它try-catch就这么简单。

进入细节：如果没有关于应用程序架构的更多信息，实际上不可能评估这种或那种方法的优点，但这里有一些一般性建议：

  • 甚至在调用之前检查先决条件refundOrder。确保订单已成功加载；确保您将要操作的订单都是可退款的（尝试退款由于业务逻辑而无法退款的订单的目的是什么？在尝试退款之前不应该通知用户/操作员吗？） .
  • 使用多个级别的try/ catch，但可能不是全部在里面refundOrder。外部块旨在捕获任何真正意外的错误，那么refundOrder仅在内部捕获它们有什么意义呢？您不想在应用程序的其他部分也这样做吗？最里面的try/catch是必要的，这样一个不可退款的订单就不会杀死所有的过程。
于 2012-04-18T12:43:56.897 回答
1

如果您需要记录并邮寄每个异常，那么您需要这样的东西：

class RefundServiceInvoker {

   private $_orders;

   public function refundOrder() {
      try {
         $this->getOrdersFromDB();
         foreach ($this->_orders as $order) {
            try {
               $order->refund();
            } catch (MyBusinessException $e) {
               # deals with the problematic $order without stopping the loop
               $this->logError($e);
               $this->sendMailToAdmin();
            }
            $this->updateOrderStatus('refunded');
         }
      } catch(Exception $e) {
         # deals with unexpected bugs
         $this->logError($e);
         $this->sendMailToAdmin();
      }
   }
}

但是您必须在 log 和 mail 方法中放置一个 try/catch，以防止它们在服务器离线时抛出任何异常，例如。如果不这样做，$orders 循环将在日志/邮件失败时停止。

如果您只需要在refund() 方法中记录/邮寄您的业务异常，那么您需要这样的东西：

class RefundServiceInvoker {

   private $_orders;

   public function refundOrder() {
      $this->getOrdersFromDB();
      foreach ($this->_orders as $order) {
         try {
            $order->refund();
         } catch (MyBusinessException $e) {
            # deals with the problematic $order without stopping the loop
            $this->logError($e);
            $this->sendMailToAdmin();
         }
         $this->updateOrderStatus('refunded');
      }
   }
}

任何其他异常都会导致 http 500 – 内部服务器错误页面，这通常是您想要的，因为这是一个意外错误。

还有其他方法可以处理此问题，但如您所见，这取决于您的需求。

于 2012-04-18T13:12:56.590 回答
0

作为logError($e)并且sendMailToAdmin()可能是最后使用的函数，通常在 try/catch 块中，我会确保它们永远不会抛出异常。

function logError($e)
{
    try
    {
        //your logic that may throw an Exception here
    }
    catch {}
}

function sendMailToAdmin($e)
{
    try
    {
        //your logic that may throw an Exception here
    }
    catch {}
}
于 2012-04-18T12:50:03.277 回答