This is a piece of code that I was wondering if it should be refactored to make it more complied with the Clean Code practices.
This is a class which is responsible for refunding some orders made by customers.
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();
}
}
}
}
of course this code is highly simplified than my original code.
My main problem is if $order->refund();
throws an exception it will be caught and logged to DB then a mail is sent. However what if $this->logError($e);
itself throws an exception? or what if the mail server was down and an exception was thrown?
Better what if the DB was down itself and $this->getOrdersFromDB();
throws an exception?
My first solution was to wrap everything in one big 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();
}
}
}
But that would mean if one order fails then all fails!! Should I put 2 try{}catch{}
one for the whole function and the other for each order? But in this case too the functions in the catch might throw an exception that won't be caught.
Note:
The application is built using Zend framework 1.11.11.
Thanks in advance.