Failed Conditions
Push — sf/last-boss ( 53d963...58156b )
by Kiyotaka
09:54 queued 04:25
created

EmptyItemsValidator::validate()   C

Complexity

Conditions 13
Paths 84

Size

Total Lines 44

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 13
nc 84
nop 2
dl 0
loc 44
rs 6.6166
c 0
b 0
f 0

How to fix   Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
/*
4
 * This file is part of EC-CUBE
5
 *
6
 * Copyright(c) LOCKON CO.,LTD. All Rights Reserved.
7
 *
8
 * http://www.lockon.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 Doctrine\ORM\EntityManagerInterface;
17
use Eccube\Entity\ItemHolderInterface;
18
use Eccube\Entity\Order;
19
use Eccube\Service\PurchaseFlow\InvalidItemException;
20
use Eccube\Service\PurchaseFlow\ItemHolderValidator;
21
use Eccube\Service\PurchaseFlow\PurchaseContext;
22
23
class EmptyItemsValidator extends ItemHolderValidator
24
{
25
    /**
26
     * @var EntityManagerInterface
27
     */
28
    protected $entityManager;
29
30
    /**
31
     * EmptyItemsProcessor constructor.
32
     *
33
     * @param EntityManagerInterface $entityManager
34
     */
35
    public function __construct(EntityManagerInterface $entityManager)
36
    {
37
        $this->entityManager = $entityManager;
38
    }
39
40
    /**
41
     * @param ItemHolderInterface $itemHolder
42
     * @param PurchaseContext $context
43
     *
44
     * @throws InvalidItemException
45
     */
46
    protected function validate(ItemHolderInterface $itemHolder, PurchaseContext $context)
47
    {
48
        foreach ($itemHolder->getItems() as $item) {
49
            if ($item->isProduct() && $item->getQuantity() == 0) {
50
                if ($itemHolder instanceof Order) {
51
                    foreach ($itemHolder->getShippings() as $Shipping) {
52
                        $Shipping->removeOrderItem($item);
53
                    }
54
                    $itemHolder->removeOrderItem($item);
0 ignored issues
show
Compatibility introduced by
$item of type object<Eccube\Entity\ItemInterface> is not a sub-type of object<Eccube\Entity\OrderItem>. It seems like you assume a concrete implementation of the interface Eccube\Entity\ItemInterface to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
55
                } else {
56
                    $itemHolder->removeItem($item);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Eccube\Entity\ItemHolderInterface as the method removeItem() does only exist in the following implementations of said interface: Eccube\Entity\Cart.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
57
                }
58
                $this->entityManager->remove($item);
59
            }
60
        }
61
62
        if (!$itemHolder instanceof Order) {
63
            return;
64
        }
65
66
        // 受注の場合は, Shippingに紐づく商品明細がない場合はShippingも削除する.
67
        foreach ($itemHolder->getShippings() as $Shipping) {
68
            $hasProductItem = false;
69
            foreach ($Shipping->getOrderItems() as $item) {
70
                if ($item->isProduct()) {
71
                    $hasProductItem = true;
72
                    break;
73
                }
74
            }
75
76
            if (!$hasProductItem) {
77
                foreach ($Shipping->getOrderItems() as $item) {
78
                    $this->entityManager->remove($item);
79
                }
80
                $itemHolder->removeShipping($Shipping);
81
                $this->entityManager->remove($Shipping);
82
            }
83
        }
84
85
        // Shippingがなくなれば購入エラー.
86
        if (count($itemHolder->getShippings()) < 1) {
87
            $this->throwInvalidItemException('front.shopping.empty_items_error');
88
        }
89
    }
90
}
91