Toolbar::renderNormalMenu() - Code Metrics - Inspection of "Merge branch 'release' into 'master'" - bb-drummer/my-application - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( 700b0f...aafb0a )

by Björn

created 2021-02-20 02:06 UTC

Toolbar::renderNormalMenu() F

↳ Parent: Toolbar

Complexity

Conditions	34
Paths	> 20000

Size

Total Lines

136

Duplication

Lines	53
Ratio	38.97 %

Importance

Changes

Metric	Value
dl	53
loc	136
rs	0
c	0
b	0
f	0
cc	34
nc	69176
nop	9

How to fix Long Method Complexity Many Parameters

<?php
/**
 * BB's Zend Framework 2 Components
 * 
 * UI Components
 *
 * @package     [MyApplication]
 * @subpackage  BB's Zend Framework 2 Components
 * @subpackage  UI Components
 * @author      Björn Bartels <[email protected]>
 * @link        https://gitlab.bjoernbartels.earth/groups/zf2
 * @license     http://www.apache.org/licenses/LICENSE-2.0 Apache License, Version 2.0
 * @copyright   copyright (c) 2016 Björn Bartels <[email protected]>
 */

namespace UIComponents\View\Helper\Components;

use \RecursiveIteratorIterator;
use Zend\Navigation\AbstractContainer;

/**
 *
 * Helper for rendering 'Bootstrap' 
 * 
 * @see \UIComponents\View\Helper\Navigation\Menu
 */
class Toolbar extends \UIComponents\View\Helper\Navigation\Menu
{
    
    /**
     * component's tag-name
     *
     * @var string
     */
    protected $tagname = 'div';
    
    /**
     * component's class-names
     *
     * @var string
     */
    protected $classnames = 'toolbar toolbar-default callout small';
    
    /**
     * component's size attributes
     *
     * @var string|array
     */
    protected $size = '';
    
    /**
     * View helper entry point:
     * Retrieves helper and optionally sets container to operate on
     *
     * @param  string|AbstractContainer $container [optional] container to operate on
     * @return self
     */
    public function __invoke($container = 'componentnavigationhelper')
    {
        if (null !== $container) {
            $this->setContainer($container);
        }

        return (clone $this);
    }
    
    /**
     * Renders a normal menu (called from {@link renderMenu()})
     *
     * @param    AbstractContainer $container            container to render
     * @param    string            $ulClass            CSS class for first UL
     * @param    string            $indent             initial indentation
     * @param    int|null            $minDepth            minimum depth
     * @param    int|null            $maxDepth            maximum depth
     * @param    bool                $onlyActive         render only active branch?
     * @param    bool                $escapeLabels        Whether or not to escape the labels
     * @param    bool                $addClassToListItem Whether or not page class applied to <li> element
     * @param    string            $liActiveClass        CSS class for active LI
     * @return string
     */
    protected function renderNormalMenu(
        \Zend\Navigation\AbstractContainer $container,
        $ulClass,
        $indent,
        $minDepth,
        $maxDepth,
        $onlyActive,
        $escapeLabels,
        $addClassToListItem,
        $liActiveClass
    ) {
        $html = '';

        // find deepest active
        $found = $this->findActive($container, $minDepth, $maxDepth);
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        /* @var $escaper \Zend\View\Helper\EscapeHtmlAttr */
        $escaper = $this->view->plugin('escapeHtmlAttr');
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.
    }
}

        if ($found) {

            $foundPage    = $found['page'];
            $foundDepth = $found['depth'];
        } else {
            $foundPage = null;
        }

        // create iterator
        $iterator = new RecursiveIteratorIterator(
            $container,
            RecursiveIteratorIterator::SELF_FIRST
        );
        if (is_int($maxDepth)) {
            $iterator->setMaxDepth($maxDepth);
        }

        // iterate container
        $prevDepth = -1;
        foreach ($iterator as $page) {
            $depth = $iterator->getDepth();
            $page->set('level', $depth);
            $isActive = $page->isActive(true);
            if ($depth < $minDepth || !$this->accept($page)) {

                // page is below minDepth or not accepted by acl/visibility
                continue;
            } elseif ($onlyActive && !$isActive) {
                // page is not active itself, but might be in the active branch
                $accept = false;
                if ($foundPage) {
                    if ($foundPage->hasPage($page)) {
                        // accept if page is a direct child of the active page
                        $accept = true;
                    } elseif ($foundPage->getParent()->hasPage($page)) {
                        // page is a sibling of the active page...
                        if (!$foundPage->hasPages(!$this->renderInvisible) ||
                            is_int($maxDepth) && $foundDepth + 1 > $maxDepth) {
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

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

    // $x is potentially undefined here.
    echo $x;
}
                            // accept if active page has no children, or the
                            // children are too deep to be rendered
                            $accept = true;
                        }
                    }
                }

                if (!$accept) {
                    continue;
                }
            }

            // make sure indentation is correct
            $depth -= $minDepth;
            $myIndent = $indent . str_repeat('    ', $depth);

            if ($depth > $prevDepth) {
                // start new ul tag
                $ulClass = '' . 
                    ($depth == 0 ? $this->getUlClass() : 
                            ($depth == 1 ? $this->getSubUlClassLevel1() : $this->getSubUlClass())
                    ) . 
                    ' level_' . $depth . 
                '';
                if ($ulClass && $depth ==    0) {

                    $ulClass = ' class="' . $escaper($ulClass) . '"';
                } else {
                    $ulClass = ' class="' . $escaper($ulClass) . '"';
                }
                $html .= $myIndent . '<' . $this->getTagname() . $ulClass . '>' . PHP_EOL;
            } elseif ($prevDepth > $depth) {
                // close li/ul tags until we're at current depth
                for ($i = $prevDepth; $i > $depth; $i--) {

                    $ind = $indent . str_repeat('        ', $i);
                    //$html .= $ind . '    </li>' . PHP_EOL;
                    $html .= $ind . '</' . $this->getTagname() . '>' . PHP_EOL;
                }
                // close previous li tag
                //$html .= $myIndent . '    </li>' . PHP_EOL;
            } else {
                // close previous li tag
                //$html .= $myIndent . '    </li>' . PHP_EOL;
            }

            // render li tag and page
            $liClasses = [];
            // Is page active?
            if ($isActive) {
                $liClasses[] = $liActiveClass;
            }
            if (!empty($this->getDefaultLiClass())) {
                $liClasses[] = $this->getDefaultLiClass();
            }
            $isBelowMaxLevel = ($maxDepth > $depth) || ($maxDepth === null) || ($maxDepth === false);
            if (!empty($page->pages) && $isBelowMaxLevel) {

                $liClasses[] = ($depth == 0 ? $this->getSubLiClassLevel0() : $this->getSubLiClass());
            }
            // Add CSS class from page to <li>
            if ($addClassToListItem && $page->getClass()) {
                $liClasses[] = $page->getClass();
            }
            $liClass = empty($liClasses) ? '' : ' class="' . $escaper(implode(' ', $liClasses)) . '"';
$myVar = 'Value';
$higher = false;

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

            $html .= /* $myIndent . '    <li' . $liClass . '>' . PHP_EOL
                . */ $myIndent . '        ' . $this->htmlify($page, $escapeLabels, $addClassToListItem) . PHP_EOL;

            // store as previous depth for next iteration
            $prevDepth = $depth;
        }

        if ($html) {

            // done iterating container; close open ul/li tags
            for ($i = $prevDepth+1; $i > 0; $i--) {
                $myIndent = $indent . str_repeat('        ', $i-1);
                $html .= /*$myIndent . '    </li>' . PHP_EOL
                    . */ $myIndent . '</' . $this->getTagname() . '>' . PHP_EOL;
            }
            $html = rtrim($html, PHP_EOL);
        }

        return $html;
    }
    
    //
    // component related getters/setters
    //
    
    /**
     * @return the $tagname
     */
    public function getTagname() {
        return $this->tagname;
    }

    /**
     * @param string $tagname
     */
    public function setTagname($tagname) {
        if ( null !== $tagname ) {
            $this->tagname = $tagname;
        }
        return $this;
    }

}

Push — master ( 700b0f...aafb0a )

Toolbar::renderNormalMenu() F

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity Many Parameters

Long Method

Many Parameters

Available Fixes

Available Fixes

1		<?php
2		/**
3		* BB's Zend Framework 2 Components
4		*
5		* UI Components
6		*
7		* @package [MyApplication]
8		* @subpackage BB's Zend Framework 2 Components
9		* @subpackage UI Components
10		* @author Björn Bartels <[email protected]>
11		* @link https://gitlab.bjoernbartels.earth/groups/zf2
12		* @license http://www.apache.org/licenses/LICENSE-2.0 Apache License, Version 2.0
13		* @copyright copyright (c) 2016 Björn Bartels <[email protected]>
14		*/
15
16		namespace UIComponents\View\Helper\Components;
17
18		use \RecursiveIteratorIterator;
19		use Zend\Navigation\AbstractContainer;
20
21		/**
22		*
23		* Helper for rendering 'Bootstrap'
24		*
25		* @see \UIComponents\View\Helper\Navigation\Menu
26		*/
27		class Toolbar extends \UIComponents\View\Helper\Navigation\Menu
28		{
29
30		/**
31		* component's tag-name
32		*
33		* @var string
34		*/
35		protected $tagname = 'div';
36
37		/**
38		* component's class-names
39		*
40		* @var string
41		*/
42		protected $classnames = 'toolbar toolbar-default callout small';
43
44		/**
45		* component's size attributes
46		*
47		* @var string\|array
48		*/
49		protected $size = '';
50
51		/**
52		* View helper entry point:
53		* Retrieves helper and optionally sets container to operate on
54		*
55		* @param string\|AbstractContainer $container [optional] container to operate on
56		* @return self
57		*/
58		public function __invoke($container = 'componentnavigationhelper')
59		{
60		if (null !== $container) {
61		$this->setContainer($container);
62		}
63
64		return (clone $this);
65		}
66
67		/**
68		* Renders a normal menu (called from {@link renderMenu()})
69		*
70		* @param AbstractContainer $container container to render
71		* @param string $ulClass CSS class for first UL
72		* @param string $indent initial indentation
73		* @param int\|null $minDepth minimum depth
74		* @param int\|null $maxDepth maximum depth
75		* @param bool $onlyActive render only active branch?
76		* @param bool $escapeLabels Whether or not to escape the labels
77		* @param bool $addClassToListItem Whether or not page class applied to <li> element
78		* @param string $liActiveClass CSS class for active LI
79		* @return string
80		*/
81		protected function renderNormalMenu(
82		\Zend\Navigation\AbstractContainer $container,
83		$ulClass,
84		$indent,
85		$minDepth,
86		$maxDepth,
87		$onlyActive,
88		$escapeLabels,
89		$addClassToListItem,
90		$liActiveClass
91		) {
92		$html = '';
93
94		// find deepest active
95		$found = $this->findActive($container, $minDepth, $maxDepth);
		0 ignored issues – show Documentation introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report `$container` is of type `object<Zend\Navigation\AbstractContainer>`, but the function expects a `object<UIComponents\View...AbstractContainer>\|null`. 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		/* @var $escaper \Zend\View\Helper\EscapeHtmlAttr */
97		$escaper = $this->view->plugin('escapeHtmlAttr');
		0 ignored issues – show Bug introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Zend\View\Renderer\RendererInterface` as the method `plugin()` does only exist in the following implementations of said interface: `Zend\View\Renderer\PhpRenderer`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
98
99	View Code Duplication	if ($found) {
		0 ignored issues – show Bug Best Practice introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report The expression `$found` of type `array` is implicitly converted to a boolean; are you sure this is intended? If so, consider using `! empty($expr)` instead to make it clear that you intend to check for an array without elements. This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent. Consider making the comparison explicit by using `empty(..)` or `! empty(...)` instead. Loading history... Duplication introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
100		$foundPage = $found['page'];
101		$foundDepth = $found['depth'];
102		} else {
103		$foundPage = null;
104		}
105
106		// create iterator
107		$iterator = new RecursiveIteratorIterator(
108		$container,
109		RecursiveIteratorIterator::SELF_FIRST
110		);
111		if (is_int($maxDepth)) {
112		$iterator->setMaxDepth($maxDepth);
113		}
114
115		// iterate container
116		$prevDepth = -1;
117		foreach ($iterator as $page) {
118		$depth = $iterator->getDepth();
119		$page->set('level', $depth);
120		$isActive = $page->isActive(true);
121	View Code Duplication	if ($depth < $minDepth \|\| !$this->accept($page)) {
		0 ignored issues – show Duplication introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
122		// page is below minDepth or not accepted by acl/visibility
123		continue;
124		} elseif ($onlyActive && !$isActive) {
125		// page is not active itself, but might be in the active branch
126		$accept = false;
127		if ($foundPage) {
128		if ($foundPage->hasPage($page)) {
129		// accept if page is a direct child of the active page
130		$accept = true;
131		} elseif ($foundPage->getParent()->hasPage($page)) {
132		// page is a sibling of the active page...
133		if (!$foundPage->hasPages(!$this->renderInvisible) \|\|
134		is_int($maxDepth) && $foundDepth + 1 > $maxDepth) {
		0 ignored issues – show Bug introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report The variable `$foundDepth` 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 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; } } 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; } 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...
135		// accept if active page has no children, or the
136		// children are too deep to be rendered
137		$accept = true;
138		}
139		}
140		}
141
142		if (!$accept) {
143		continue;
144		}
145		}
146
147		// make sure indentation is correct
148		$depth -= $minDepth;
149		$myIndent = $indent . str_repeat(' ', $depth);
150
151		if ($depth > $prevDepth) {
152		// start new ul tag
153		$ulClass = '' .
154		($depth == 0 ? $this->getUlClass() :
155		($depth == 1 ? $this->getSubUlClassLevel1() : $this->getSubUlClass())
156		) .
157		' level_' . $depth .
158		'';
159	View Code Duplication	if ($ulClass && $depth == 0) {
		0 ignored issues – show Duplication introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
160		$ulClass = ' class="' . $escaper($ulClass) . '"';
161		} else {
162		$ulClass = ' class="' . $escaper($ulClass) . '"';
163		}
164		$html .= $myIndent . '<' . $this->getTagname() . $ulClass . '>' . PHP_EOL;
165		} elseif ($prevDepth > $depth) {
166		// close li/ul tags until we're at current depth
167	View Code Duplication	for ($i = $prevDepth; $i > $depth; $i--) {
		0 ignored issues – show Duplication introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
168		$ind = $indent . str_repeat(' ', $i);
169		//$html .= $ind . ' </li>' . PHP_EOL;
170		$html .= $ind . '</' . $this->getTagname() . '>' . PHP_EOL;
171		}
172		// close previous li tag
173		//$html .= $myIndent . ' </li>' . PHP_EOL;
174		} else {
175		// close previous li tag
176		//$html .= $myIndent . ' </li>' . PHP_EOL;
177		}
178
179		// render li tag and page
180		$liClasses = [];
181		// Is page active?
182		if ($isActive) {
183		$liClasses[] = $liActiveClass;
184		}
185		if (!empty($this->getDefaultLiClass())) {
186		$liClasses[] = $this->getDefaultLiClass();
187		}
188		$isBelowMaxLevel = ($maxDepth > $depth) \|\| ($maxDepth === null) \|\| ($maxDepth === false);
189	View Code Duplication	if (!empty($page->pages) && $isBelowMaxLevel) {
		0 ignored issues – show Duplication introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
190		$liClasses[] = ($depth == 0 ? $this->getSubLiClassLevel0() : $this->getSubLiClass());
191		}
192		// Add CSS class from page to <li>
193		if ($addClassToListItem && $page->getClass()) {
194		$liClasses[] = $page->getClass();
195		}
196		$liClass = empty($liClasses) ? '' : ' class="' . $escaper(implode(' ', $liClasses)) . '"';
		0 ignored issues – show Unused Code introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report `$liClass` 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...
197
198		$html .= /* $myIndent . ' <li' . $liClass . '>' . PHP_EOL
199		. */ $myIndent . ' ' . $this->htmlify($page, $escapeLabels, $addClassToListItem) . PHP_EOL;
200
201		// store as previous depth for next iteration
202		$prevDepth = $depth;
203		}
204
205	View Code Duplication	if ($html) {
		0 ignored issues – show Duplication introduced 2016-08-09 09:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
206		// done iterating container; close open ul/li tags
207		for ($i = $prevDepth+1; $i > 0; $i--) {
208		$myIndent = $indent . str_repeat(' ', $i-1);
209		$html .= /*$myIndent . ' </li>' . PHP_EOL
210		. */ $myIndent . '</' . $this->getTagname() . '>' . PHP_EOL;
211		}
212		$html = rtrim($html, PHP_EOL);
213		}
214
215		return $html;
216		}
217
218		//
219		// component related getters/setters
220		//
221
222		/**
223		* @return the $tagname
224		*/
225		public function getTagname() {
226		return $this->tagname;
227		}
228
229		/**
230		* @param string $tagname
231		*/
232		public function setTagname($tagname) {
233		if ( null !== $tagname ) {
234		$this->tagname = $tagname;
235		}
236		return $this;
237		}
238
239		}

bb-drummer / my-application

Push — master ( 700b0f...aafb0a )

Toolbar::renderNormalMenu() F

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity Many Parameters

Long Method

Many Parameters

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like