问题描述
我有一个准备收据输出的功能。但是由于它具有各种条件,因此最终变得很长且难以理解。
该如何重构呢?有什么想法吗?
如果我将其拆分为100个小函数,那真的会更好吗?
public static function prepare_receipt($printer)
{
if (self::hasItems($printer['id']))
{
$output = '';
if ($_POST['pre_receipt'])
{
$output .= "======== Pre receipt =======\n\n\n";
}
/**
* Time and table
*/
if ($_POST['isTakeaway'] || $_POST["isDeliveryGuys"] || $_POST["isBolt"]) {
$output .= "Table: " . $_POST['table'] . "\n";
$output .= "Floor: " . $_POST['floor'] . "\n";
$output .= "Time: " . $_POST['takeawayTime'] . "\n";
if ($_POST['order_comment']) {
$output .= "Comment: " . removeSpecialChars($_POST['order_comment']) . "\n";
}
} else {
$output .= "Table: " . $_POST['table'] . "\n\n";
$output .= "Floor: " . $_POST['floor'] . "\n\n";
if ($_POST['order_comment']) {
$output .= "Comment: " . removeSpecialChars($_POST['order_comment']) . "\n";
}
}
$output .= "------------------------\n";
/**
* Food items
*/
foreach ($_POST['orderedItems'] as $orderedItem)
{
$has_unprinted_quantity = false;
if (isset($orderedItem['last_printed_quantity'])) {
$unprinted_quantity_count = intval($orderedItem['is_printed_quantity']) - intval($orderedItem['last_printed_quantity']);
if ($unprinted_quantity_count > 0) {
$has_unprinted_quantity = true;
}
}
if ( ($orderedItem['should_print'] &&
!$orderedItem['is_printed'] &&
$orderedItem['is_visible']) ||
$_POST['pre_receipt'] ||
$has_unprinted_quantity)
{
if (is_array($orderedItem['printers'])) {
$in_printer = in_array($printer['id'],$orderedItem['printers']);
} else {
$in_printer = in_array($printer['id'],json_decode($orderedItem['printers'],true));
}
if ( $in_printer || $_POST['pre_receipt'] )
{
if ($orderedItem['is_sidedish'] && !$_POST['pre_receipt']) {
continue;
}
if ($has_unprinted_quantity) {
$output .= $unprinted_quantity_count . 'x ';
} else {
$output .= $orderedItem['quantity'] . 'x ';
}
// We ned to split it for multiple lines...
$itemDescriptionParts = self::split($orderedItem['description']);
foreach ($itemDescriptionParts as $itemDescription) {
$itemDescriptionClean = removeSpecialChars($itemDescription);
$output .= $itemDescriptionClean;
}
// Add price for pre receipt
if ($_POST['pre_receipt']) {
$output .= " - " . number_format($orderedItem['price_with_discount'],2,'.',',');
}
if (!$_POST['pre_receipt']) {
if ($orderedItem['comments'] != '') {
$output .= " > " . removeSpecialChars(substr($orderedItem['comments'],27)) . "\n";
}
}
/** Side dishes */
if (isset($orderedItem['side_dishes']) && !$_POST['pre_receipt'])
{
foreach ($orderedItem['side_dishes'] as $side_dish) {
$output .= "\n + " . removeSpecialChars(substr($side_dish['description'],27)) . "\n";
}
}
$output .= "\n";
}
}
}
/**
* Sums
*/
/**
* Footer
*/
$output .= "------------------------\n";
if ($_POST['pre_receipt'])
{
$output .= "\nSubtotal: " . number_format($_POST['order']['subtotal'],') . "\n";
$output .= "discount: " . number_format($_POST['order']['discount'],') . "\n";
$output .= "Total: " . number_format($_POST['order']['total'],') . "\n\n";
}
$output .= "Time: " . getTime() . "\n";
return $output;
}
else
{
return 'EMPTY';
}
}
任何朝着正确方向的指针将不胜感激。
解决方法
如果遵循语义,那么重构通常可以很好地工作。在您的情况下:您已经对不同部分进行了评论。这通常是功能本身的标志。
只是给您一个想法:之后的样子:
$output .= create_headline(...);
$output .= create_time_table(...);
$output .= create_separator();
foreach ($_POST['orderedItems'] as $orderedItem) {
$output .= create_food_item($orderedItem,$_POST['pre_receipt'],...);
}
$output .= create_separator();
$output .= create_footer(...);
这将节省在收据的某个区域中查找错误的时间。
,我会建议https://en.wikipedia.org/wiki/Divide-and-conquer_algorithm,并且您的函数已经有注释,该注释指示如何将此函数分为具有单一职责的多个部分。
我也建议不要直接使用$ _POST,所有输入数据必须始终经过验证并可能被过滤。输入的数据应作为依赖关系传递,以遵守依赖关系注入,有关其他良好实践,请参见https://phptherightway.com/。
我也将避免使用字符串连接,将所有部分存储在数组中,然后使用分隔符将它们合并/插入。
,巧妙地使用三元运算符查看代码,并将orderitem循环转换为其他函数,可以将代码长度大幅减少一半。不需要为每个操作(例如打印头,打印尾巴等)创建功能,因为这些打印中的逻辑非常简单,并且如果有大量不需要的功能,您的代码可能会变得混乱且难以导航。您可以执行以下操作。另请注意使用。字符串连接的(。)运算符使字符串的可读性降低,因此更喜欢使用{}运算符打印变量。
<?php
public static function prepare_receipt($printer)
{
if (self::hasItems($printer['id']))
{
$output = isset($_POST['pre_receipt']) ? "======== Pre receipt =======\n\n\n" : "" ;
//addiing time table
$output .= "Table: {$_POST['table']}. \n Floor: {$_POST['floor']}. \n";
//adding time if it is takeaway or isDeliveryGuys or isBolt
$output .= ($_POST['isTakeaway'] || $_POST["isDeliveryGuys"] || $_POST["isBolt"]) ? "Time: {$_POST['takeawayTime']}. \n" : "" ;
//adding order comment
$output .= $_POST['order_comment']) ? "Comment: {removeSpecialChars($_POST['order_comment'])} \n" : "" ;
//print order items
this->getOrderItems($_POST[orderedItems],&$output);
// footer
$output .= "------------------------\n";
if ($_POST['pre_receipt'])
$output .= "\nSubtotal: {number_format($_POST['order']['subtotal'],2,'.',',')} \n Discount: { number_format($_POST['order']['discount'],') } \n Total: {number_format($_POST['order']['total'],')} \n\n";
$output .= "Time: " . getTime() . "\n";
return $output;
}
else
{
return 'EMPTY';
}
}
?>
。