Issues in DeliveryFeeFreeByShippingPreprocessor.php - All Issues - Inspection of "Merge pull request #4203 from hiro-t59/convert_acc..." - EC-CUBE/ec-cube - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 4.0 ( 87d096...bcc1be )

by Kiyotaka

created 2019-07-03 06:57 UTC

DeliveryFeeFreeByShippingPreprocessor.php (4 issues)

Labels

Severity

Major 4

Chihiro Adachi 4

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

/*
 * This file is part of EC-CUBE
 *
 * Copyright(c) EC-CUBE CO.,LTD. All Rights Reserved.
 *
 * http://www.ec-cube.co.jp/
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Eccube\Service\PurchaseFlow\Processor;

use Eccube\Entity\BaseInfo;
use Eccube\Entity\ItemHolderInterface;
use Eccube\Entity\Order;
use Eccube\Repository\BaseInfoRepository;
use Eccube\Service\PurchaseFlow\ItemHolderPreprocessor;
use Eccube\Service\PurchaseFlow\PurchaseContext;

/**
 * 送料無料条件を適用する.
 * お届け先ごとに条件判定を行う.
 */
class DeliveryFeeFreeByShippingPreprocessor implements ItemHolderPreprocessor
{
    /**
     * @var BaseInfo
     */
    protected $BaseInfo;

    /**
     * DeliveryFeeProcessor constructor.
     *
     * @param BaseInfoRepository $baseInfoRepository
     */
    public function __construct(BaseInfoRepository $baseInfoRepository)
    {
        $this->BaseInfo = $baseInfoRepository->get();
    }

    /**
     * @param ItemHolderInterface $itemHolder
     * @param PurchaseContext $context
     */
    public function process(ItemHolderInterface $itemHolder, PurchaseContext $context)
    {
        if (!($this->BaseInfo->getDeliveryFreeAmount() || $this->BaseInfo->getDeliveryFreeQuantity())) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
            return;
        }

        // Orderの場合はお届け先ごとに判定する.
        if ($itemHolder instanceof Order) {
            /** @var Order $Order */
            $Order = $itemHolder;
            foreach ($Order->getShippings() as $Shipping) {
                $isFree = false;
                $total = 0;
                $quantity = 0;
                foreach ($Shipping->getProductOrderItems() as $Item) {
                    $total += $Item->getPriceIncTax() * $Item->getQuantity();
                    $quantity += $Item->getQuantity();
                }
                // 送料無料（金額）を超えている
                if ($this->BaseInfo->getDeliveryFreeAmount()) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                    if ($total >= $this->BaseInfo->getDeliveryFreeAmount()) {
                        $isFree = true;
                    }
                }
                // 送料無料（個数）を超えている
                if ($this->BaseInfo->getDeliveryFreeQuantity()) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
                    if ($quantity >= $this->BaseInfo->getDeliveryFreeQuantity()) {
                        $isFree = true;
                    }
                }
                if ($isFree) {
                    foreach ($Shipping->getOrderItems() as $Item) {
                        if ($Item->getProcessorName() == DeliveryFeePreprocessor::class) {
                            $Item->setQuantity(0);
                        }
                    }
                }
            }
        }
    }
}


1		<?php
2
3		/*
4		* This file is part of EC-CUBE
5		*
6		* Copyright(c) EC-CUBE CO.,LTD. All Rights Reserved.
7		*
8		* http://www.ec-cube.co.jp/
9		*
10		* For the full copyright and license information, please view the LICENSE
11		* file that was distributed with this source code.
12		*/
13
14		namespace Eccube\Service\PurchaseFlow\Processor;
15
16		use Eccube\Entity\BaseInfo;
17		use Eccube\Entity\ItemHolderInterface;
18		use Eccube\Entity\Order;
19		use Eccube\Repository\BaseInfoRepository;
20		use Eccube\Service\PurchaseFlow\ItemHolderPreprocessor;
21		use Eccube\Service\PurchaseFlow\PurchaseContext;
22
23		/**
24		* 送料無料条件を適用する.
25		* お届け先ごとに条件判定を行う.
26		*/
27		class DeliveryFeeFreeByShippingPreprocessor implements ItemHolderPreprocessor
28		{
29		/**
30		* @var BaseInfo
31		*/
32		protected $BaseInfo;
33
34		/**
35		* DeliveryFeeProcessor constructor.
36		*
37		* @param BaseInfoRepository $baseInfoRepository
38		*/
39	70	public function __construct(BaseInfoRepository $baseInfoRepository)
40		{
41	70	$this->BaseInfo = $baseInfoRepository->get();
42		}
43
44		/**
45		* @param ItemHolderInterface $itemHolder
46		* @param PurchaseContext $context
47		*/
48	62	public function process(ItemHolderInterface $itemHolder, PurchaseContext $context)
49		{
50	62	if (!($this->BaseInfo->getDeliveryFreeAmount() \|\| $this->BaseInfo->getDeliveryFreeQuantity())) {
		0 ignored issues – show Bug Best Practice introduced 2018-06-26 09:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$this->BaseInfo->getDeliveryFreeAmount()` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history... Bug Best Practice introduced 2018-06-26 09:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$this->BaseInfo->getDeliveryFreeQuantity()` of type `integer\|null` is loosely compared to `false`; this is ambiguous if the integer can be zero. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
51	52	return;
52		}
53
54		// Orderの場合はお届け先ごとに判定する.
55	10	if ($itemHolder instanceof Order) {
56		/** @var Order $Order */
57	10	$Order = $itemHolder;
58	10	foreach ($Order->getShippings() as $Shipping) {
59	10	$isFree = false;
60	10	$total = 0;
61	10	$quantity = 0;
62	10	foreach ($Shipping->getProductOrderItems() as $Item) {
63	10	$total += $Item->getPriceIncTax() * $Item->getQuantity();
64	10	$quantity += $Item->getQuantity();
65		}
66		// 送料無料（金額）を超えている
67	10	if ($this->BaseInfo->getDeliveryFreeAmount()) {
		0 ignored issues – show Bug Best Practice introduced 2018-06-26 09:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$this->BaseInfo->getDeliveryFreeAmount()` of type `string\|null` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
68	5	if ($total >= $this->BaseInfo->getDeliveryFreeAmount()) {
69	3	$isFree = true;
70		}
71		}
72		// 送料無料（個数）を超えている
73	10	if ($this->BaseInfo->getDeliveryFreeQuantity()) {
		0 ignored issues – show Bug Best Practice introduced 2018-06-26 09:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$this->BaseInfo->getDeliveryFreeQuantity()` of type `integer\|null` is loosely compared to `true`; this is ambiguous if the integer can be zero. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
74	5	if ($quantity >= $this->BaseInfo->getDeliveryFreeQuantity()) {
75	3	$isFree = true;
76		}
77		}
78	10	if ($isFree) {
79	6	foreach ($Shipping->getOrderItems() as $Item) {
80	6	if ($Item->getProcessorName() == DeliveryFeePreprocessor::class) {
81	10	$Item->setQuantity(0);
82		}
83		}
84		}
85		}
86		}
87		}
88		}
89

EC-CUBE / ec-cube

Push — 4.0 ( 87d096...bcc1be )

DeliveryFeeFreeByShippingPreprocessor.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like