如何重构一个长函数？

Question

我有一个准备收据输出的功能。但是由于它具有各种条件，因此最终会变得很长且难以理解..

如何重构这个？有什么想法吗？

如果我把它分成 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'], 0, 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'], 0, 27)) . "\n";
                        }
                    }

                    $output .= "\n";
                }
            }
        }

        /**
         * Sums
         */


        /**
         * Footer
         */
        $output .= "------------------------\n";

        if ($_POST['pre_receipt'])
        {
            $output .= "\nSubtotal: " . number_format($_POST['order']['subtotal'], 2, '.', ',') . "\n";
            $output .= "Discount: " . number_format($_POST['order']['discount'], 2, '.', ',') . "\n";
            $output .= "Total: " . number_format($_POST['order']['total'], 2, '.', ',') . "\n\n";
        }

        $output .= "Time: " . getTime() . "\n";

        return $output;
    }
    else
    {
        return 'EMPTY';
    }
}

任何正确方向的指针将不胜感激。

Answer 1

如果遵循语义，重构通常效果很好。在您的情况下：您已经为不同的部分制作了 cmets。这通常是其自身功能的标志。

只是给你一个想法：之后会是什么样子：

$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(...);

这将在搜索收据的特定区域中的错误时节省时间。

Answer 2

我建议https://en.wikipedia.org/wiki/Divide-and-conquer_algorithm，并且您的函数已经有注释，表明该函数如何被划分为具有单一职责的多个。

我还建议不要直接使用 $_POST，所有输入数据都必须始终经过验证并可能进行过滤。来自输入的数据应作为依赖项传递，以遵守依赖项注入，请参阅 https://phptherightway.com/ 了解其他良好做法。

我也会避免使用字符串连接，将所有部分存储在一个数组中，然后使用分隔符连接/分解它们。

Answer 3

查看您的代码，巧妙地使用三元运算符并将 orderitem 循环转换为不同的函数，可以将您的代码长度大幅减少一半。无需为打印头、打印尾等每个操作创建函数，因为这些打印中的逻辑非常简单，如果有大量不需要的函数，您的代码可能会很混乱且难以导航。您可以执行以下操作。另请注意使用 .用于字符串连接的 (dot) 运算符会降低字符串的可读性，因此更喜欢使用 {} 运算符打印变量。

    <?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'], 2, '.', ',') } \n Total: {number_format($_POST['order']['total'], 2, '.', ',')} \n\n";
            $output .= "Time: " . getTime() . "\n";

            return $output;
        }
        else
        {
            return 'EMPTY';
        }
    }

    ?>

.