Updatecart::preparePage() - Code Metrics - Inspection of "fixes to coding style" - HaaseIT/HCSF - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 8cdaf4...db6695 )

by Marcus

created 2017-03-22 07:58 UTC

Updatecart::preparePage() D

↳ Parent: Updatecart

Complexity

Conditions	21
Paths	37

Size

Total Lines	85
Code Lines	57

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
dl	0
loc	85
rs	4.7769
c	1
b	0
f	0
cc	21
eloc	57
nc	37
nop	0

How to fix Long Method Complexity

<?php

/*
    HCSF - A multilingual CMS and Shopsystem
    Copyright (C) 2014  Marcus Haase - [email protected]

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.

    This program is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    GNU General Public License for more details.

    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

namespace HaaseIT\HCSF\Controller\Shop;

use HaaseIT\HCSF\HelperConfig;

/**
 * Class Updatecart
 * @package HaaseIT\HCSF\Controller\Shop
 */
class Updatecart extends Base
{
    /**
     *
     */
    public function preparePage()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $this->P = new \HaaseIT\HCSF\CorePage($this->serviceManager);
        $this->P->cb_pagetype = 'content';

        if (
            (
                HelperConfig::$shop["show_pricesonlytologgedin"]
                && !\HaaseIT\HCSF\Customer\Helper::getUserData()
            )
            || !isset($_SERVER["HTTP_REFERER"])
        ) {
            $this->P->oPayload->cl_html = $this->serviceManager->get('textcats')->T("denied_default");
        } else {
            $iAmount = '';
            if (isset($_REQUEST["amount"])) {
                $iAmount = $_REQUEST["amount"];
            }

            if (!isset($_REQUEST["itemno"]) || $_REQUEST["itemno"] == '' || !is_numeric($iAmount)) {
                $this->replyToCartUpdate('noitemnooramount');
            } else {
                $iAmount = floor($iAmount);

                // Check if this item exists
                $aData = $this->serviceManager->get('oItem')->sortItems('', $_REQUEST["itemno"]);
                if (!isset($aData)) {
                    $this->replyToCartUpdate('itemnotfound');
                } else {
                    // build the key for this item for the shoppingcart
                    $sItemno = $aData["item"][$_REQUEST["itemno"]]['itm_no'];
                    $sCartKey = $sItemno;

                    if (isset(HelperConfig::$shop["custom_order_fields"])) {
                        foreach (HelperConfig::$shop["custom_order_fields"] as $sValue) {
                            if (isset($aData["item"][$sItemno]["itm_data"][$sValue])) {
                                $aOptions = [];
                                $TMP = explode('|', $aData["item"][$sItemno]["itm_data"][$sValue]);
                                foreach ($TMP as $sTMPValue) {
                                    if (trim($sTMPValue) != '') {
                                        $aOptions[] = $sTMPValue;
                                    }
                                }
                                unset($sTMP);

                                if (isset($_REQUEST[$sValue]) && in_array($_REQUEST[$sValue], $aOptions)) {
                                    $sCartKey .= '|' . $sValue . ':' . $_REQUEST[$sValue];
                                } else {
                                    $this->replyToCartUpdate('requiredfieldmissing');
                                }
                            }
                        }
                    }
                    // if this Items is not in cart and amount is 0, no need to do anything, return to referer
                    if (!isset($_SESSION["cart"][$sCartKey]) && $iAmount == 0) {
                        $this->replyToCartUpdate('noactiontaken');
                    }
                    $aItem = [
                        'amount' => $iAmount,
                        'price' => $this->serviceManager->get('oItem')->calcPrice($aData["item"][$sItemno]),
                        'rg' => $aData["item"][$sItemno]['itm_rg'],
                        'vat' => $aData["item"][$sItemno]['itm_vatid'],
                        'name' => $aData["item"][$sItemno]['itm_name'],
                        'img' => $aData["item"][$sItemno]['itm_img'],
                    ];
                    if (isset($_SESSION["cart"][$sCartKey])) { // if this item is already in cart, update amount
                        if ($iAmount == 0) { // new amount == 0 -> remove from cart
                            unset($_SESSION["cart"][$sCartKey]);
                            if (count($_SESSION["cart"]) == 0) { // once the last cart item is unset, we no longer need cartpricesums
                                unset($_SESSION["cartpricesums"]);
                            }
                            $this->replyToCartUpdate('removed', ['cartkey' => $sCartKey]);
                        } else { // update amount
                            $_SESSION["cart"][$sCartKey]["amount"] = $iAmount;
                            $this->replyToCartUpdate('updated', ['cartkey' => $sCartKey, 'amount' => $iAmount]);
                        }
                    } else { // if this item is not in the cart yet, add it
                        $_SESSION["cart"][$sCartKey] = $aItem;
                    }
                    $this->replyToCartUpdate('added', ['cartkey' => $sCartKey, 'amount' => $iAmount]);
                }
            }
            die();

        }
    }

    /**
     * @param $sReply
     * @param array $aMore
     */
    private function replyToCartUpdate($sReply, $aMore = []) {
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
        if (isset($_REQUEST["ajax"])) {
            $aAR = [
                'cart' => $_SESSION["cart"],
                'reply' => $sReply,
                'cartsums' => \HaaseIT\HCSF\Shop\Helper::calculateCartItems($_SESSION["cart"]),
                'currency' => HelperConfig::$shop["waehrungssymbol"],
                'numberformat_decimals' => HelperConfig::$core['numberformat_decimals'],
                'numberformat_decimal_point' => HelperConfig::$core['numberformat_decimal_point'],
                'numberformat_thousands_seperator' => HelperConfig::$core['numberformat_thousands_seperator'],
            ];
            if (count($aMore)) {
                $aAR = array_merge($aAR, $aMore);
            }
            echo $this->serviceManager->get('twig')->render('shop/update-cart.twig', $aAR);
        } else {
            $aMSG["msg"] =  $sReply;
foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}
            if (count($aMore)) {
                $aMSG = array_merge($aMSG, $aMore);
            }
            header('Location: '.\HaaseIT\Toolbox\Tools::makeLinkHRefWithAddedGetVars($_SERVER["HTTP_REFERER"], $aMSG, true, false));
        }
        die();

    }

}

1			<?php
2
3			/*
4			HCSF - A multilingual CMS and Shopsystem
5			Copyright (C) 2014 Marcus Haase - [email protected]
6
7			This program is free software: you can redistribute it and/or modify
8			it under the terms of the GNU General Public License as published by
9			the Free Software Foundation, either version 3 of the License, or
10			(at your option) any later version.
11
12			This program is distributed in the hope that it will be useful,
13			but WITHOUT ANY WARRANTY; without even the implied warranty of
14			MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15			GNU General Public License for more details.
16
17			You should have received a copy of the GNU General Public License
18			along with this program. If not, see <http://www.gnu.org/licenses/>.
19			*/
20
21			namespace HaaseIT\HCSF\Controller\Shop;
22
23			use HaaseIT\HCSF\HelperConfig;
24
25			/**
26			* Class Updatecart
27			* @package HaaseIT\HCSF\Controller\Shop
28			*/
29			class Updatecart extends Base
30			{
31			/**
32			*
33			*/
34			public function preparePage()
			0 ignored issues – show Coding Style introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `preparePage` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `preparePage` uses the super-global variable `$_REQUEST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `preparePage` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
35			{
36			$this->P = new \HaaseIT\HCSF\CorePage($this->serviceManager);
37			$this->P->cb_pagetype = 'content';
38
39			if (
40			(
41			HelperConfig::$shop["show_pricesonlytologgedin"]
42			&& !\HaaseIT\HCSF\Customer\Helper::getUserData()
43			)
44			\|\| !isset($_SERVER["HTTP_REFERER"])
45			) {
46			$this->P->oPayload->cl_html = $this->serviceManager->get('textcats')->T("denied_default");
47			} else {
48			$iAmount = '';
49			if (isset($_REQUEST["amount"])) {
50			$iAmount = $_REQUEST["amount"];
51			}
52
53			if (!isset($_REQUEST["itemno"]) \|\| $_REQUEST["itemno"] == '' \|\| !is_numeric($iAmount)) {
54			$this->replyToCartUpdate('noitemnooramount');
55			} else {
56			$iAmount = floor($iAmount);
57
58			// Check if this item exists
59			$aData = $this->serviceManager->get('oItem')->sortItems('', $_REQUEST["itemno"]);
60			if (!isset($aData)) {
61			$this->replyToCartUpdate('itemnotfound');
62			} else {
63			// build the key for this item for the shoppingcart
64			$sItemno = $aData["item"][$_REQUEST["itemno"]]['itm_no'];
65			$sCartKey = $sItemno;
66
67			if (isset(HelperConfig::$shop["custom_order_fields"])) {
68			foreach (HelperConfig::$shop["custom_order_fields"] as $sValue) {
69			if (isset($aData["item"][$sItemno]["itm_data"][$sValue])) {
70			$aOptions = [];
71			$TMP = explode('\|', $aData["item"][$sItemno]["itm_data"][$sValue]);
72			foreach ($TMP as $sTMPValue) {
73			if (trim($sTMPValue) != '') {
74			$aOptions[] = $sTMPValue;
75			}
76			}
77			unset($sTMP);
78
79			if (isset($_REQUEST[$sValue]) && in_array($_REQUEST[$sValue], $aOptions)) {
80			$sCartKey .= '\|' . $sValue . ':' . $_REQUEST[$sValue];
81			} else {
82			$this->replyToCartUpdate('requiredfieldmissing');
83			}
84			}
85			}
86			}
87			// if this Items is not in cart and amount is 0, no need to do anything, return to referer
88			if (!isset($_SESSION["cart"][$sCartKey]) && $iAmount == 0) {
89			$this->replyToCartUpdate('noactiontaken');
90			}
91			$aItem = [
92			'amount' => $iAmount,
93			'price' => $this->serviceManager->get('oItem')->calcPrice($aData["item"][$sItemno]),
94			'rg' => $aData["item"][$sItemno]['itm_rg'],
95			'vat' => $aData["item"][$sItemno]['itm_vatid'],
96			'name' => $aData["item"][$sItemno]['itm_name'],
97			'img' => $aData["item"][$sItemno]['itm_img'],
98			];
99			if (isset($_SESSION["cart"][$sCartKey])) { // if this item is already in cart, update amount
100			if ($iAmount == 0) { // new amount == 0 -> remove from cart
101			unset($_SESSION["cart"][$sCartKey]);
102			if (count($_SESSION["cart"]) == 0) { // once the last cart item is unset, we no longer need cartpricesums
103			unset($_SESSION["cartpricesums"]);
104			}
105			$this->replyToCartUpdate('removed', ['cartkey' => $sCartKey]);
106			} else { // update amount
107			$_SESSION["cart"][$sCartKey]["amount"] = $iAmount;
108			$this->replyToCartUpdate('updated', ['cartkey' => $sCartKey, 'amount' => $iAmount]);
109			}
110			} else { // if this item is not in the cart yet, add it
111			$_SESSION["cart"][$sCartKey] = $aItem;
112			}
113			$this->replyToCartUpdate('added', ['cartkey' => $sCartKey, 'amount' => $iAmount]);
114			}
115			}
116			die();
			0 ignored issues – show Coding Style Compatibility introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report The method `preparePage()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
117			}
118			}
119
120			/**
121			* @param $sReply
122			* @param array $aMore
123			*/
124			private function replyToCartUpdate($sReply, $aMore = []) {
			0 ignored issues – show Coding Style introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `replyToCartUpdate` uses the super-global variable `$_REQUEST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `replyToCartUpdate` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `replyToCartUpdate` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
125			if (isset($_REQUEST["ajax"])) {
126			$aAR = [
127			'cart' => $_SESSION["cart"],
128			'reply' => $sReply,
129			'cartsums' => \HaaseIT\HCSF\Shop\Helper::calculateCartItems($_SESSION["cart"]),
130			'currency' => HelperConfig::$shop["waehrungssymbol"],
131			'numberformat_decimals' => HelperConfig::$core['numberformat_decimals'],
132			'numberformat_decimal_point' => HelperConfig::$core['numberformat_decimal_point'],
133			'numberformat_thousands_seperator' => HelperConfig::$core['numberformat_thousands_seperator'],
134			];
135			if (count($aMore)) {
136			$aAR = array_merge($aAR, $aMore);
137			}
138			echo $this->serviceManager->get('twig')->render('shop/update-cart.twig', $aAR);
139			} else {
140			$aMSG["msg"] = $sReply;
			0 ignored issues – show Coding Style Comprehensibility introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report `$aMSG` was never initialized. Although not strictly required by PHP, it is generally a good practice to add `$aMSG = array();` before regardless. Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code. Let’s take a look at an example: foreach ($collection as $item) { $myArray['foo'] = $item->getFoo(); if ($item->hasBar()) { $myArray['bar'] = $item->getBar(); } // do something with $myArray } As you can see in this example, the array `$myArray` is initialized the first time when the foreach loop is entered. You can also see that the value of the `bar` key is only written conditionally; thus, its value might result from a previous iteration. This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization `$myArray = array()` either outside or inside the foreach loop. Loading history...
141			if (count($aMore)) {
142			$aMSG = array_merge($aMSG, $aMore);
143			}
144			header('Location: '.\HaaseIT\Toolbox\Tools::makeLinkHRefWithAddedGetVars($_SERVER["HTTP_REFERER"], $aMSG, true, false));
145			}
146			die();
			0 ignored issues – show Coding Style Compatibility introduced 2017-03-21 09:14 UTC by Report Bug Copy Issue Report The method `replyToCartUpdate()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
147			}
148
149			}

HaaseIT / HCSF

Push — master ( 8cdaf4...db6695 )

Updatecart::preparePage() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like