CartForm::updatecart() - Code Metrics - Inspection of "Wrap all order extension hooks in try/catch blocks..." - silvershop/silvershop-core - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#557)

by Roman

created 2016-10-11 11:26 UTC

CartForm::updatecart() D

↳ Parent: CartForm

Complexity

Conditions	24
Paths	128

Size

Total Lines	81
Code Lines	49

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	0
CRAP Score	600

Importance

Changes

Metric	Value
dl	0
loc	81
ccs	0
cts	56
cp	0
rs	4.6335
c	0
b	0
f	0
cc	24
eloc	49
nc	128
nop	2
crap	600

How to fix Long Method Complexity

<?php

/**
 * Renders the cart inside a form, so that it is editable.
 *
 * @package shop
 */
class CartForm extends Form
{
    protected $cart;

    public function __construct($controller, $name = "CartForm", $cart = null, $template = "Cart")
    {
        $this->cart = $cart;
        $fields = FieldList::create(
            CartEditField::create("Items", "", $this->cart)
                ->setTemplate($template)
        );
        $actions = FieldList::create(
            FormAction::create("updatecart", _t('CartForm.UpdateCart', "Update Cart"))
        );

        parent::__construct($controller, $name, $fields, $actions);
    }

    /**
     * Update the cart using data collected
     */
    public function updatecart($data, $form)

    {
        $items = $this->cart->Items();
        $updatecount = $removecount = 0;

        $request = $this->getRequest();
        $order = ShoppingCart::curr();
        if ($request && $request->isAjax() && $order) {
            ShopTools::install_locale($order->Locale);
        }

        $numericConverter = NumericField::create('_temp');

        $messages = [];
        $badMessages = [];
        if (isset($data['Items']) && is_array($data['Items'])) {
            foreach ($data['Items'] as $itemid => $fields) {
                $item = $items->byID($itemid);
                if (!$item) {
                    continue;
                }
                //delete lines
                if (isset($fields['Remove']) || (isset($fields['Quantity']) && (int)$fields['Quantity'] <= 0)) {
                    if (ShoppingCart::singleton()->remove($item->Buyable())) {
                        $removecount++;
                    } else {
                        $badMessages[] = ShoppingCart::singleton()->getMessage();
                    }

                    continue;
                }
                //update quantities
                if (isset($fields['Quantity']) && $quantity = Convert::raw2sql($fields['Quantity'])) {
                    $numericConverter->setValue($quantity);
                    if (!ShoppingCart::singleton()->setQuantity($item->Buyable(), $numericConverter->dataValue())) {
                        $badMessages[] = ShoppingCart::singleton()->getMessage();
                    }
                }
                //update variations
                if (isset($fields['ProductVariationID']) && $id = Convert::raw2sql($fields['ProductVariationID'])) {
                    if ($item->ProductVariationID != $id) {
                        $item->ProductVariationID = $id;
                    }
                }
                //TODO: make updates through ShoppingCart class
                //TODO: combine with items that now match exactly
                //TODO: validate changes
                if ($item->isChanged()) {
                    $item->write();
                    $updatecount++;
                }
            }
        }
        if ($removecount) {
            $messages['remove'] = _t(
                'CartForm.REMOVED_ITEMS',
                "Removed {count} items.",
                "count is the amount that was removed",
                array('count' => $removecount)
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            );
        }
        if ($updatecount) {
            $messages['updatecount'] = _t(
                'CartForm.UPDATED_ITEMS',
                "Updated {count} items.",
                "count is the amount that was updated",
                array('count' => $updatecount)
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            );
        }
        if (count($messages)) {
            $form->sessionMessage(implode(" ", $messages), "good");
        }

        if (count($badMessages)) {
            $form->sessionMessage(implode(" ", $badMessages), "bad");
        }

        $this->extend('updateCartFormResponse', $request, $response, $form);

        return $response ? $response : $this->controller->redirectBack();
    }
}


1			<?php
2
3			/**
4			* Renders the cart inside a form, so that it is editable.
5			*
6			* @package shop
7			*/
8			class CartForm extends Form
9			{
10			protected $cart;
11
12			public function __construct($controller, $name = "CartForm", $cart = null, $template = "Cart")
13			{
14			$this->cart = $cart;
15			$fields = FieldList::create(
16			CartEditField::create("Items", "", $this->cart)
17			->setTemplate($template)
18			);
19			$actions = FieldList::create(
20			FormAction::create("updatecart", _t('CartForm.UpdateCart', "Update Cart"))
21			);
22
23			parent::__construct($controller, $name, $fields, $actions);
24			}
25
26			/**
27			* Update the cart using data collected
28			*/
29			public function updatecart($data, $form)
			0 ignored issues – show Complexity introduced 2016-02-23 23:20 UTC by Report Bug Copy Issue Report This operation has 103360 execution paths which exceeds the configured maximum of 200. A high number of execution paths generally suggests many nested conditional statements and make the code less readible. This can usually be fixed by splitting the method into several smaller methods. You can also find more information in the “Code” section of your repository. Loading history...
30			{
31			$items = $this->cart->Items();
32			$updatecount = $removecount = 0;
33
34			$request = $this->getRequest();
35			$order = ShoppingCart::curr();
36			if ($request && $request->isAjax() && $order) {
37			ShopTools::install_locale($order->Locale);
38			}
39
40			$numericConverter = NumericField::create('_temp');
41
42			$messages = [];
43			$badMessages = [];
44			if (isset($data['Items']) && is_array($data['Items'])) {
45			foreach ($data['Items'] as $itemid => $fields) {
46			$item = $items->byID($itemid);
47			if (!$item) {
48			continue;
49			}
50			//delete lines
51			if (isset($fields['Remove']) \|\| (isset($fields['Quantity']) && (int)$fields['Quantity'] <= 0)) {
52			if (ShoppingCart::singleton()->remove($item->Buyable())) {
53			$removecount++;
54			} else {
55			$badMessages[] = ShoppingCart::singleton()->getMessage();
56			}
57
58			continue;
59			}
60			//update quantities
61			if (isset($fields['Quantity']) && $quantity = Convert::raw2sql($fields['Quantity'])) {
62			$numericConverter->setValue($quantity);
63			if (!ShoppingCart::singleton()->setQuantity($item->Buyable(), $numericConverter->dataValue())) {
64			$badMessages[] = ShoppingCart::singleton()->getMessage();
65			}
66			}
67			//update variations
68			if (isset($fields['ProductVariationID']) && $id = Convert::raw2sql($fields['ProductVariationID'])) {
69			if ($item->ProductVariationID != $id) {
70			$item->ProductVariationID = $id;
71			}
72			}
73			//TODO: make updates through ShoppingCart class
74			//TODO: combine with items that now match exactly
75			//TODO: validate changes
76			if ($item->isChanged()) {
77			$item->write();
78			$updatecount++;
79			}
80			}
81			}
82			if ($removecount) {
83			$messages['remove'] = _t(
84			'CartForm.REMOVED_ITEMS',
85			"Removed {count} items.",
86			"count is the amount that was removed",
87			array('count' => $removecount)
			0 ignored issues – show Documentation introduced 2016-02-23 23:20 UTC by Report Bug Copy Issue Report `array('count' => $removecount)` is of type `array<string,integer,{"count":"integer"}>`, but the function expects a `string`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
88			);
89			}
90			if ($updatecount) {
91			$messages['updatecount'] = _t(
92			'CartForm.UPDATED_ITEMS',
93			"Updated {count} items.",
94			"count is the amount that was updated",
95			array('count' => $updatecount)
			0 ignored issues – show Documentation introduced 2016-02-23 23:20 UTC by Report Bug Copy Issue Report `array('count' => $updatecount)` is of type `array<string,integer,{"count":"integer"}>`, but the function expects a `string`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
96			);
97			}
98			if (count($messages)) {
99			$form->sessionMessage(implode(" ", $messages), "good");
100			}
101
102			if (count($badMessages)) {
103			$form->sessionMessage(implode(" ", $badMessages), "bad");
104			}
105
106			$this->extend('updateCartFormResponse', $request, $response, $form);
107
108			return $response ? $response : $this->controller->redirectBack();
109			}
110			}
111

silvershop / silvershop-core

Pull Request — master (#557)

CartForm::updatecart() D

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like