Completed
Push — menus ( 04517d...ba40fa )
by Arnaud
02:08
created

MenusCreate::process()   C

Complexity

Conditions 9
Paths 2

Size

Total Lines 86

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 86
rs 6.7498
c 0
b 0
f 0
cc 9
nc 2
nop 0

How to fix   Long Method   

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
 * Copyright (c) Arnaud Ligny <[email protected]>
4
 *
5
 * For the full copyright and license information, please view the LICENSE
6
 * file that was distributed with this source code.
7
 */
8
9
namespace Cecil\Step;
10
11
use Cecil\Collection\Menu\Collection as MenusCollection;
12
use Cecil\Collection\Menu\Entry;
13
use Cecil\Collection\Menu\Menu;
14
use Cecil\Collection\Page\Page;
15
16
/**
17
 * Generates menus.
18
 */
19
class MenusCreate extends AbstractStep
20
{
21
    /**
22
     * @var MenusCollection
23
     */
24
    protected $menus;
25
26
    /**
27
     * {@inheritdoc}
28
     */
29
    public function process()
30
    {
31
        // create menus collection, with a 'main' menu
32
        $main = new Menu('main');
33
        $this->menus = new MenusCollection('menus');
34
        $this->menus->add($main);
35
36
        // add entries from pages to menus collection
37
        $this->collectPages();
38
39
        /*
40
         * Removing/adding/replacing menus entries from config
41
         * ie:
42
         *   site:
43
         *     menu:
44
         *       main:
45
         *         example:
46
         *           name: "Example"
47
         *           url: https://example.com
48
         *           weight: 999
49
         *         a-propos:
50
         *           id: about
51
         *           enabled: false
52
         */
53
        if ($this->builder->getConfig()->get('site.menu')) {
54
            call_user_func_array($this->builder->getMessageCb(), ['MENU', 'Creating menus (config)']);
55
            $menus = $this->builder->getConfig()->get('site.menu');
56
            $totalConfig = array_sum(array_map('count', $menus));
57
            $countConfig = 0;
58
            foreach ($menus as $menu => $entry) {
59
                /* @var $menu \Cecil\Collection\Menu\Menu */
60
                if (!$this->menus->has($menu)) {
61
                    $this->menus->add(new Menu($menu));
62
                }
63
                $menu = $this->menus->get($menu);
64
                foreach ($entry as $key => $property) {
65
                    $updated = '';
0 ignored issues
show
Unused Code introduced by
$updated is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
66
                    $countConfig++;
67
68
                    // ID is key
69
                    if (!array_key_exists('id', $property)) {
70
                        $property['id'] = $key;
71
                    }
72
73
                    // is entry already exist?
74
                    if ($menu->has($property['id'])) {
75
                        // remove a disabled entry
76
                        if (array_key_exists('enabled', $property) && false === $property['enabled']) {
77
                            call_user_func_array($this->builder->getMessageCb(), [
78
                                'MENU_PROGRESS',
79
                                sprintf('%s > %s (removed)', $menu, $property['id']),
80
                                $countConfig,
81
                                $totalConfig
82
                            ]);
83
                            $menu->remove($property['id']);
84
                            continue;
85
                        }
86
                        // merge properties
87
                        $current = $menu->get($property['id'])->toArray();
88
                        $property = array_merge($current, $property);
89
                        call_user_func_array($this->builder->getMessageCb(), [
90
                            'MENU_PROGRESS',
91
                            sprintf('%s > %s (updated)', $menu, $property['id']),
92
                            $countConfig,
93
                            $totalConfig
94
                        ]);
95
                    } else {
96
                        call_user_func_array($this->builder->getMessageCb(), [
97
                            'MENU_PROGRESS',
98
                            sprintf('%s > %s', $menu, $property['id']),
99
                            $countConfig,
100
                            $totalConfig
101
                        ]);
102
                    }
103
                    // add/replace entry
104
                    $item = (new Entry($property['id']))
105
                        ->setName($property['name'] ?? ucfirst($key))
106
                        ->setUrl($property['url'] ?? '/noURL')
107
                        ->setWeight($property['weight'] ?? 0);
108
                    $menu->add($item);
109
                }
110
            }
111
        }
112
113
        $this->builder->setMenus($this->menus);
114
    }
115
116
    /**
117
     * Collects pages with a menu variable.
118
     */
119
    protected function collectPages()
120
    {
121
        $count = 0;
122
123
        $filteredPages = $this->builder->getPages()
124
            ->filter(function (Page $page)
125
        {
126
            if ($page->getVariable('menu')) {
127
                return true;
128
            }
129
        });
130
131
        $total = count($filteredPages);
132
133
        if ($total > 0) {
134
            call_user_func_array($this->builder->getMessageCb(), ['MENU', 'Creating menus (pages)']);
135
        }
136
137
        /* @var $page \Cecil\Collection\Page\Page */
138
        foreach ($filteredPages as $page) {
139
            $count++;
140
            /*
141
             * Single case
142
             * ie:
143
             *   menu: main
144
             */
145
            if (is_string($page->getVariable('menu'))) {
146
                $item = (new Entry($page->getId()))
147
                    ->setName($page->getVariable('title'))
148
                    ->setUrl($page->getUrl());
149
                /* @var $menu \Cecil\Collection\Menu\Menu */
150
                $menu = $page->getVariable('menu');
151
                if (!$this->menus->has($menu)) {
152
                    $this->menus->add(new Menu($menu));
153
                }
154
                $this->menus->get($menu)->add($item);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Cecil\Collection\ItemInterface as the method add() does only exist in the following implementations of said interface: Cecil\Collection\Menu\Menu, Cecil\Collection\Taxonomy\Term, Cecil\Collection\Taxonomy\Vocabulary.

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...
155
            } else {
156
                /*
157
                 * Multiple case
158
                 * ie:
159
                 *   menu:
160
                 *     main:
161
                 *       weight: 999
162
                 *     other
163
                 */
164
                if (is_array($page->getVariable('menu'))) {
165
                    foreach ($page->getVariable('menu') as $menu => $property) {
166
                        $item = (new Entry($page->getId()))
167
                            ->setName($page->getVariable('title'))
168
                            ->setUrl($page->getId())
169
                            ->setWeight($property['weight']);
170
                        /* @var $menu \Cecil\Collection\Menu\Menu */
171
                        if (!$this->menus->has($menu)) {
172
                            $this->menus->add(new Menu($menu));
173
                        }
174
                        $this->menus->get($menu)->add($item);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Cecil\Collection\ItemInterface as the method add() does only exist in the following implementations of said interface: Cecil\Collection\Menu\Menu, Cecil\Collection\Taxonomy\Term, Cecil\Collection\Taxonomy\Vocabulary.

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...
175
                    }
176
                }
177
            }
178
            call_user_func_array($this->builder->getMessageCb(), [
179
                'MENU_PROGRESS',
180
                sprintf('%s > %s', $menu, $page->getId()),
0 ignored issues
show
Bug introduced by
The variable $menu does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
181
                $count,
182
                $total
183
            ]);
184
        }
185
    }
186
}
187