How would you refactor this function?

@martinzeltin heres my first pass:

<?php

class Printer
{
    public static function prepare_receipt(Request $request, $printer)
    {
        if (self::hasItems($printer['id'])) {
            $output = '';

            if ($request->pre_receipt) {
                $output .= "======== Pre receipt =======\n\n\n";
            }

            /**
             * Time and table
             */

            $output .= "Table: " . $request->table . "\n\n";
            $output .= "Floor: " . $request->floor . "\n\n";

            if ($request->isTakeaway || $request->isDeliveryGuys || $request->isBolt) {
                $output .= "Time: " . $request->takeawayTime . "\n";
            }

            if ($request->order_comment) {
                $output .= "Comment: " . removeSpecialChars($request->order_comment) . "\n";
            }

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


            /**
             * Food items
             */
            foreach ($request->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']) ||
                    $request->pre_receipt ||
                    $has_unprinted_quantity) {
                    
                    $printers = (is_array($orderedItem['printers'])) ? $orderedItem['printers'] : json_decode($orderedItem['printers'],
                        true);
                    
                    if (in_array($printer['id'], $printers) || $request->pre_receipt) {

                        $output .= ($has_unprinted_quantity) ? $unprinted_quantity_count . 'x ' : $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 ($request->pre_receipt) {
                            $output .= " - " . number_format($orderedItem['price_with_discount'], 2, '.', ',');
                        }

                        if (!$request->pre_receipt) {
                            if ($orderedItem['comments'] != '') {
                                $output .= "   > " . removeSpecialChars(substr($orderedItem['comments'], 0, 27)) . "\n";
                            }
                        }

                        /** Side dishes */
                        if (isset($orderedItem['side_dishes']) && !$request->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 ($request->pre_receipt) {
                $output .= "\nSubtotal: " . number_format($request->order['subtotal'], 2, '.', ',') . "\n";
                $output .= "Discount: " . number_format($request->order['discount'], 2, '.', ',') . "\n";
                $output .= "Total: " . number_format($request->order['total'], 2, '.', ',') . "\n\n";
            }

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

            return $output;
        }
        return 'EMPTY';
    }
    
}

notable changes:

• use $request instead of $_POST so you can sanitize your inputs before using them.
• this bit

	if ($_POST['isTakeaway'] || $_POST["isDeliveryGuys"] || $_POST["isBolt"]) {

only related to displaying

$output .= "Time: " . $_POST['takeawayTime'] . "\n";

so I've dried that up.

this:

                    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 || $request->pre_receipt) {

can be simplified to:

	$printers = is_array($orderedItem['printers']) ? $orderedItem['printers'] : json_decode($orderedItem['printers'], true);
                    
         if (in_array($printer['id'], $printers) || $request->pre_receipt) {

theres some more that can be done, so I might update again..

Methods, methods methods.. Take each little bit of logic and move to a method.

public static function prepare_receipt(Request $request, $printer)
    {
        if (self::hasItems($printer['id'])) {
            $output = '';
            $output .= self::makePreReciept();
            $output .= self::prependLabels();
            $output .= self::addTimes();
            $output .= self::addComments();
            //etc
     }
}

private function makePreReciept()
{
	if ($_POST['pre_receipt'])
		{
			return "======== Pre receipt =======\n\n\n";
		}
         return '';
}

@sinnbeck In the static method you can use $this->, just static:: or self::

@michaloravec Yeah totally missed that static keyword. Thanks!

Edit: fixed

@sinnbeck next pass move to methods.

This:

$orderedItem['should_print'] &&
                        !$orderedItem['is_printed'] &&
                        $orderedItem['is_visible']) ||
                    $request->pre_receipt ||
                    $has_unprinted_quantity

would benefit from having a named method that describes what these 5 conditions are checking

@automica Totally. That is really hard to read. A well named methods is much better.

@sinnbeck I really like your example, very clean. One question though.. Shouldn't there be a foreach loop to print all itmes?

Put that loop into a method, and have each piece of the loop inside other methods as well :)

@sinnbeck

...and have each piece of the loop inside other methods as well :)

I'm not sure I understand. You mean to repeat the loop in every method? Wouldn't that be duplicate code? Wouldn't it be better to have one loop outside the methods?

@martinzeltin you've got a comment

      /**
             * Food items
             */

and this would indicate you could move that entire loop into a method called 'addFoodItems'.

any bits you have already commented, could be individual methods, like how @sinnbeck has suggested.

the key for a good refactor is to reduce the level of indentation so you only have 1 level deep. Anything lower than that can be pushed to a separate method.

I would recommend reading https://github.com/object-calisthenics/phpcs-calisthenics-rules

knowing these rules, will help you sniff out when you need to refactor.

Before going wild on a refactor though, make sure you have a test case set up that you can run after every small change you make, so you can ensure your method still works. That will stop you having to unravel broken code at a later date.

Thanks guys! I'll try to split it into individual methods and see how it goes!

full of html code, breaking MVC pattern? , try to prepare RAW data, also you can create helper to do some lightwork

put the $request->order['subtotal'] to array, let the frontend render ?

$output .= "\nSubtotal: " . number_format($request->order['subtotal'], 2, '.', ',') . "\n";

@martinzeltin Oh yeah. You can see an example of how I dynammically write to a file here (Well prepare the content). I have a file writer that I call and then call set methods on

https://github.com/sinnbeck/laravel-served/blob/master/src/Images/MysqlImage.php#L23

https://github.com/sinnbeck/laravel-served/blob/master/src/Docker/DockerFileBuilder.php