CategoryController::indexAction() - Code Metrics - speckcommerce/SpeckCatalog - Measure and Improve Code Quality continuously with Scrutinizer

CategoryController::indexAction() B
last analyzed 2016-03-01 16:40 UTC

↳ Parent: CategoryController

Complexity

Conditions	2
Paths	2

Size

Total Lines	25
Code Lines	15

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	0
CRAP Score	6

Importance

Changes	2
Bugs	1	Features	1

Metric	Value
c	2
b	1
f	1
dl	0
loc	25
ccs	0
cts	20
cp	0
rs	8.8571
cc	2
eloc	15
nc	2
nop	0
crap	6

<?php

namespace SpeckCatalog\Controller;

use Zend\Mvc\Controller\AbstractActionController;
use Zend\View\Model\ViewModel;
use Exception;

class CategoryController extends AbstractActionController
{
    protected $catalogService;

    public function indexAction()
    {
        $paginatorVars = $this->getPaginatorVars();

        $id       = $this->params('id');
        $service  = $this->getServiceLocator()->get('speckcatalog_category_service');
        $category = $service->getCategoryForView($id, $paginatorVars);

        $crumbs = $service->getCrumbs($category);
        $this->layout()->crumbs = array(
            'type' => 'category',
            'crumbs' => $crumbs,
        );

        if (null === $category) {
            throw new \Exception('fore oh fore');
        }

        $paginatorVars['categoryId'] = $category->getCategoryId();

        return new ViewModel(array(
            'category' => $category,
            'paginatorVars' => $paginatorVars,
        ));
    }

    public function setCatalogService($catalogService)
    {
        $this->catalogService = $catalogService;
    }

    public function getCatalogService()
    {
        return $this->catalogService;
    }

    public function getPaginatorVars($stringify = false)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        if (isset($_GET['nn'])) {
            $this->perPageAction();
        }
        $keys = array('n', 'p', 'o', 's');
        $paginatorVars = array();
        foreach ($keys as $key) {
            if (isset($_GET[$key])) {
                $paginatorVars[$key] = $_GET[$key];
            }
        }

        return $paginatorVars;
    }

    public function perPageAction()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $keys = array('o', 's');
        foreach ($keys as $key) {
            if (isset($_GET[$key])) {
                $paginatorVars[$key] = $_GET[$key];
foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}
            }
        }

        $page = (isset($_GET['p']) ? $_GET['p'] : 1);
        $perPage = (isset($_GET['n']) ? $_GET['n'] : 10);

        $offset = ((($page * $perPage) - $perPage) + 1); //this is the first item on the page

        $paginatorVars['p'] = floor($offset / $_GET['nn']) + 1; //new page number
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

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

    // $x is potentially undefined here.
    echo $x;
}
        $paginatorVars['n'] = $_GET['nn'];                      //new items per page

        $query = '?' . http_build_query($paginatorVars);

        return $this->redirect()->toUrl('/category/' . $this->params('id') . $query);
    }
}


1			<?php
2
3			namespace SpeckCatalog\Controller;
4
5			use Zend\Mvc\Controller\AbstractActionController;
6			use Zend\View\Model\ViewModel;
7			use Exception;
8
9			class CategoryController extends AbstractActionController
10			{
11			protected $catalogService;
12
13			public function indexAction()
14			{
15			$paginatorVars = $this->getPaginatorVars();
16
17			$id = $this->params('id');
18			$service = $this->getServiceLocator()->get('speckcatalog_category_service');
19			$category = $service->getCategoryForView($id, $paginatorVars);
20
21			$crumbs = $service->getCrumbs($category);
22			$this->layout()->crumbs = array(
23			'type' => 'category',
24			'crumbs' => $crumbs,
25			);
26
27			if (null === $category) {
28			throw new \Exception('fore oh fore');
29			}
30
31			$paginatorVars['categoryId'] = $category->getCategoryId();
32
33			return new ViewModel(array(
34			'category' => $category,
35			'paginatorVars' => $paginatorVars,
36			));
37			}
38
39			public function setCatalogService($catalogService)
40			{
41			$this->catalogService = $catalogService;
42			}
43
44			public function getCatalogService()
45			{
46			return $this->catalogService;
47			}
48
49			public function getPaginatorVars($stringify = false)
			0 ignored issues – show Unused Code introduced 2015-11-24 10:42 UTC by Report Bug Copy Issue Report The parameter `$stringify` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history... Coding Style introduced 2015-11-24 10:42 UTC by Report Bug Copy Issue Report `getPaginatorVars` uses the super-global variable `$_GET` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
50			{
51			if (isset($_GET['nn'])) {
52			$this->perPageAction();
53			}
54			$keys = array('n', 'p', 'o', 's');
55			$paginatorVars = array();
56			foreach ($keys as $key) {
57			if (isset($_GET[$key])) {
58			$paginatorVars[$key] = $_GET[$key];
59			}
60			}
61
62			return $paginatorVars;
63			}
64
65			public function perPageAction()
			0 ignored issues – show Coding Style introduced 2015-11-24 10:42 UTC by Report Bug Copy Issue Report `perPageAction` uses the super-global variable `$_GET` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
66			{
67			$keys = array('o', 's');
68			foreach ($keys as $key) {
69			if (isset($_GET[$key])) {
70			$paginatorVars[$key] = $_GET[$key];
			0 ignored issues – show Coding Style Comprehensibility introduced 2015-11-24 10:42 UTC by Report Bug Copy Issue Report `$paginatorVars` was never initialized. Although not strictly required by PHP, it is generally a good practice to add `$paginatorVars = array();` before regardless. Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code. Let’s take a look at an example: foreach ($collection as $item) { $myArray['foo'] = $item->getFoo(); if ($item->hasBar()) { $myArray['bar'] = $item->getBar(); } // do something with $myArray } As you can see in this example, the array `$myArray` is initialized the first time when the foreach loop is entered. You can also see that the value of the `bar` key is only written conditionally; thus, its value might result from a previous iteration. This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization `$myArray = array()` either outside or inside the foreach loop. Loading history...
71			}
72			}
73
74			$page = (isset($_GET['p']) ? $_GET['p'] : 1);
75			$perPage = (isset($_GET['n']) ? $_GET['n'] : 10);
76
77			$offset = ((($page * $perPage) - $perPage) + 1); //this is the first item on the page
78
79			$paginatorVars['p'] = floor($offset / $_GET['nn']) + 1; //new page number
			0 ignored issues – show Bug introduced 2015-11-24 10:42 UTC by Report Bug Copy Issue Report The variable `$paginatorVars` 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...
80			$paginatorVars['n'] = $_GET['nn']; //new items per page
81
82			$query = '?' . http_build_query($paginatorVars);
83
84			return $this->redirect()->toUrl('/category/' . $this->params('id') . $query);
85			}
86			}
87

speckcommerce / SpeckCatalog

CategoryController::indexAction() B last analyzed 2016-03-01 16:40 UTC

Complexity

Size

Duplication

Code Coverage

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like

CategoryController::indexAction() B
last analyzed 2016-03-01 16:40 UTC