ShortListController - Code Metrics - Inspection of "Correcting Shortlist controller to ensure adding c..." - salted-herring/silverstripe-shortlist - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 86c958...d453af )

by Simon

created 2016-04-15 07:16 UTC

ShortListController B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	247
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	5

Importance

Changes	9
Bugs	0	Features	1

Metric	Value
wmc	46
c	9
b	0
f	1
lcom	1
cbo	5
dl	0
loc	247
rs	8.3999

10 Methods

Rating	Name	Size	Complexity
A	init()	10	2
B	index()	22	4
A	Link()	11	2
C	renderList()	27	7
C	addToShortList()	34	7
C	addOrRemove()	47	11
B	removeFromShortList()	18	5
B	shortListCount()	14	6
A	getSessionShortList()	4	1
A	getShortListSession()	4	1

How to fix Complexity

<?php

class ShortListController extends Page_Controller
{
    private static $allowed_actions = array(
        'renderList',
        'addOrRemove'
    );

    private static $url_handlers = array(
        'add'       => 'addOrRemove',
        'remove'    => 'addOrRemove',
        '$URL!'     => 'renderList',
    );

    private static $extensions = array(
        'ShortListPaginationExtension'
    );

    public function init()
    {
        parent::init();

        Session::start();

        if ($this->request->getVar('page')) {
            $this->currentPage = $this->request->getVar('page');
        }
    }

    /**
     * When landing on the homepage, if there is a shortlist for the current
     * user, redirect to the correct URL. Otherwise, 404.
     * */
    public function index($request)
    {
        if (($shortlist = $this->getSessionShortList())) {
            return $this->redirect(Config::inst()->get('ShortList', 'URLSegment') . $shortlist->URL);
        } else {
            /*
if (!ShortList::isBrowser()) {
                return $this->httpError(404);
            }
*/

            $shortlist = $this->getSessionShortList();

            if (!$shortlist || !$shortlist->exists()) {
                $shortlist = new ShortList();
                $shortlist->write();
            }
        }

        // render with empty template.
        return $this->renderWith(array('Page', 'ShortList_empty'));
    }

    /**
     * Get the absolute URL of this controller.
     * */
    public function Link($action = null)
    {
        $shortlist = $this->getSessionShortList();
        $url = Config::inst()->get('ShortList', 'URLSegment');

        if ($shortlist) {
            $url .= $shortlist->URL;
        }

        return $url;
    }

    public function renderList($request)
    {
        $shortlist = DataObject::get_one('ShortList', $filter = array('URL' => $request->param('URL')));

        if (is_null(session_id()) ||
            !$request->param('URL') ||
            !$shortlist ||
            !$shortlist->exists()
        ) {
            return $this->httpError(404);
        }

        $link = false;
        $count = 0;

        if ($shortlist && $shortlist->exists()) {
            $link = $shortlist->Link();
            $count = $shortlist->ShortListItems()->Count();
        }

        return $this->customise(array(
            'ShortlistURL' => $link,
            'ShortlistCount' => $count
        ))->renderWith(
            array('ShortList', 'Page')
        );
    }

    /**
     * Add an item to the shortlist.
     *
     * @param ID id of the object to add.
     * @param type classname of the item to remove
     * @param session session id.
     *
     * */
    public function addToShortList($ID = false, $type = null, $session = false)
    {
        if (!$ID || is_null($type) || !$session) {
            return false;
        }

        $shortlist = $this->getSessionShortList();

        if (!$shortlist || !$shortlist->exists()) {
            $shortlist = new ShortList();
            $shortlist->SessionID = $session;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
            $shortlist->write();
        }

        // check whether the itme is already in the list
        // before attempting to add it.
        $existing = $shortlist->ShortListItems()->filterAny(
            array('ItemID' => $ID, 'ItemType' => $type)
        );

        if ($existing->count() == 1) {
            return true;
        }

        $shortlistItem = new ShortListItem();
        $shortlistItem->ShortListID = $shortlist->ID;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $shortlistItem->ItemID = $ID;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $shortlistItem->ItemType = $type;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

        $shortlist->ShortListItems()->add($shortlistItem);
        $shortlist->write();

        return true;
    }

    public function addOrRemove($request)
    {
        if (is_null(session_id()) ||
            !$request->getVar('id') ||
            !$request->getVar('type') ||
            !$request->getVar('s') ||
            $request->getVar('s') != session_id()
        ) {
            return $this->httpError(404);
        }

        if (strpos($request->getURL(), 'remove') !== false) {
            $status = $this->removeFromShortList(
                $ID = $request->getVar('id'),
                $type = $request->getVar('type'),
                $session = $request->getVar('s')
            );
        } else {
            $status = $this->addToShortList(
                $ID = $request->getVar('id'),
                $type = $request->getVar('type'),
                $session = $request->getVar('s')
            );
        }


        if ($request->isAjax()) {
            $shortlist = $this->getSessionShortList();
            $url = false;

            if ($shortlist && $shortlist->exists()) {
                $url = $shortlist->Link();
            }

            return json_encode(array(
                'status' => $status,
                'count' => $this->shortListCount($session),
                'url' => $url
            ));
        }

        if (array_key_exists('output', $request->getVars())) {
            return true;
        }

        return $this->redirectBack();
    }

    /**
     * Remove an item from the shortlist.
     *
     * @param ID id of the object to remove.
     * @param type classname of the item to remove
     * @param session session id.
     *
     * */
    private function removeFromShortList($ID = false, $type = null, $session = false)
    {
        $shortlist = $this->getSessionShortList();

        if (!$shortlist || !$shortlist->exists()) {
            return true;
        }

        $item = DataObject::get_one('ShortListItem', $filter = "ItemType = '" . $type . "' AND ItemID = " . $ID);

        if ($item && $item->exists()) {
            $item->delete();
        } else {
            return false;
        }

        return true;
    }

    /**
     * Get the number of items in the current short list.
     *
     * @param session The session to check & find a shortlist for.
     * @return mixed false if no session exists - else the number of items in the shortlist.
     * */
    public function shortListCount($session = false)
    {
        if (is_null(session_id()) || !$session || $session != session_id()) {
            return false;
        }

        $shortlist = $this->getSessionShortList();

        if (!$shortlist || !$shortlist->exists()) {
            return 0;
        }

        return $shortlist->Items()->count();
    }

    private function getSessionShortList()
    {
        return DataObject::get_one('ShortList', $filter = array('SessionID' => session_id()));
    }

    public static function getShortListSession()
    {
        return DataObject::get_one('ShortList', $filter = array('SessionID' => session_id()));
    }
}


1			<?php
2
3			class ShortListController extends Page_Controller
4			{
5			private static $allowed_actions = array(
6			'renderList',
7			'addOrRemove'
8			);
9
10			private static $url_handlers = array(
11			'add' => 'addOrRemove',
12			'remove' => 'addOrRemove',
13			'$URL!' => 'renderList',
14			);
15
16			private static $extensions = array(
17			'ShortListPaginationExtension'
18			);
19
20			public function init()
21			{
22			parent::init();
23
24			Session::start();
25
26			if ($this->request->getVar('page')) {
27			$this->currentPage = $this->request->getVar('page');
28			}
29			}
30
31			/**
32			* When landing on the homepage, if there is a shortlist for the current
33			* user, redirect to the correct URL. Otherwise, 404.
34			* */
35			public function index($request)
36			{
37			if (($shortlist = $this->getSessionShortList())) {
38			return $this->redirect(Config::inst()->get('ShortList', 'URLSegment') . $shortlist->URL);
39			} else {
40			/*
41			if (!ShortList::isBrowser()) {
42			return $this->httpError(404);
43			}
44			*/
45
46			$shortlist = $this->getSessionShortList();
47
48			if (!$shortlist \|\| !$shortlist->exists()) {
49			$shortlist = new ShortList();
50			$shortlist->write();
51			}
52			}
53
54			// render with empty template.
55			return $this->renderWith(array('Page', 'ShortList_empty'));
56			}
57
58			/**
59			* Get the absolute URL of this controller.
60			* */
61			public function Link($action = null)
62			{
63			$shortlist = $this->getSessionShortList();
64			$url = Config::inst()->get('ShortList', 'URLSegment');
65
66			if ($shortlist) {
67			$url .= $shortlist->URL;
68			}
69
70			return $url;
71			}
72
73			public function renderList($request)
74			{
75			$shortlist = DataObject::get_one('ShortList', $filter = array('URL' => $request->param('URL')));
76
77			if (is_null(session_id()) \|\|
78			!$request->param('URL') \|\|
79			!$shortlist \|\|
80			!$shortlist->exists()
81			) {
82			return $this->httpError(404);
83			}
84
85			$link = false;
86			$count = 0;
87
88			if ($shortlist && $shortlist->exists()) {
89			$link = $shortlist->Link();
90			$count = $shortlist->ShortListItems()->Count();
91			}
92
93			return $this->customise(array(
94			'ShortlistURL' => $link,
95			'ShortlistCount' => $count
96			))->renderWith(
97			array('ShortList', 'Page')
98			);
99			}
100
101			/**
102			* Add an item to the shortlist.
103			*
104			* @param ID id of the object to add.
105			* @param type classname of the item to remove
106			* @param session session id.
107			*
108			* */
109			public function addToShortList($ID = false, $type = null, $session = false)
110			{
111			if (!$ID \|\| is_null($type) \|\| !$session) {
112			return false;
113			}
114
115			$shortlist = $this->getSessionShortList();
116
117			if (!$shortlist \|\| !$shortlist->exists()) {
118			$shortlist = new ShortList();
119			$shortlist->SessionID = $session;
			1 ignored issue – show Documentation introduced 2016-04-15 07:18 UTC by Report Bug Copy Issue Report The property `SessionID` does not exist on `object<ShortList>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
120			$shortlist->write();
121			}
122
123			// check whether the itme is already in the list
124			// before attempting to add it.
125			$existing = $shortlist->ShortListItems()->filterAny(
126			array('ItemID' => $ID, 'ItemType' => $type)
127			);
128
129			if ($existing->count() == 1) {
130			return true;
131			}
132
133			$shortlistItem = new ShortListItem();
134			$shortlistItem->ShortListID = $shortlist->ID;
			0 ignored issues – show Documentation introduced 2016-04-15 07:18 UTC by Report Bug Copy Issue Report The property `ShortListID` does not exist on `object<ShortListItem>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
135			$shortlistItem->ItemID = $ID;
			0 ignored issues – show Documentation introduced 2016-04-15 07:18 UTC by Report Bug Copy Issue Report The property `ItemID` does not exist on `object<ShortListItem>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
136			$shortlistItem->ItemType = $type;
			1 ignored issue – show Documentation introduced 2016-04-15 07:18 UTC by Report Bug Copy Issue Report The property `ItemType` does not exist on `object<ShortListItem>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
137
138			$shortlist->ShortListItems()->add($shortlistItem);
139			$shortlist->write();
140
141			return true;
142			}
143
144			public function addOrRemove($request)
145			{
146			if (is_null(session_id()) \|\|
147			!$request->getVar('id') \|\|
148			!$request->getVar('type') \|\|
149			!$request->getVar('s') \|\|
150			$request->getVar('s') != session_id()
151			) {
152			return $this->httpError(404);
153			}
154
155			if (strpos($request->getURL(), 'remove') !== false) {
156			$status = $this->removeFromShortList(
157			$ID = $request->getVar('id'),
158			$type = $request->getVar('type'),
159			$session = $request->getVar('s')
160			);
161			} else {
162			$status = $this->addToShortList(
163			$ID = $request->getVar('id'),
164			$type = $request->getVar('type'),
165			$session = $request->getVar('s')
166			);
167			}
168
169
170			if ($request->isAjax()) {
171			$shortlist = $this->getSessionShortList();
172			$url = false;
173
174			if ($shortlist && $shortlist->exists()) {
175			$url = $shortlist->Link();
176			}
177
178			return json_encode(array(
179			'status' => $status,
180			'count' => $this->shortListCount($session),
181			'url' => $url
182			));
183			}
184
185			if (array_key_exists('output', $request->getVars())) {
186			return true;
187			}
188
189			return $this->redirectBack();
190			}
191
192			/**
193			* Remove an item from the shortlist.
194			*
195			* @param ID id of the object to remove.
196			* @param type classname of the item to remove
197			* @param session session id.
198			*
199			* */
200			private function removeFromShortList($ID = false, $type = null, $session = false)
201			{
202			$shortlist = $this->getSessionShortList();
203
204			if (!$shortlist \|\| !$shortlist->exists()) {
205			return true;
206			}
207
208			$item = DataObject::get_one('ShortListItem', $filter = "ItemType = '" . $type . "' AND ItemID = " . $ID);
209
210			if ($item && $item->exists()) {
211			$item->delete();
212			} else {
213			return false;
214			}
215
216			return true;
217			}
218
219			/**
220			* Get the number of items in the current short list.
221			*
222			* @param session The session to check & find a shortlist for.
223			* @return mixed false if no session exists - else the number of items in the shortlist.
224			* */
225			public function shortListCount($session = false)
226			{
227			if (is_null(session_id()) \|\| !$session \|\| $session != session_id()) {
228			return false;
229			}
230
231			$shortlist = $this->getSessionShortList();
232
233			if (!$shortlist \|\| !$shortlist->exists()) {
234			return 0;
235			}
236
237			return $shortlist->Items()->count();
238			}
239
240			private function getSessionShortList()
241			{
242			return DataObject::get_one('ShortList', $filter = array('SessionID' => session_id()));
243			}
244
245			public static function getShortListSession()
246			{
247			return DataObject::get_one('ShortList', $filter = array('SessionID' => session_id()));
248			}
249			}
250

salted-herring / silverstripe-shortlist

Push — master ( 86c958...d453af )

ShortListController B

Complexity

Size/Duplication

Coupling/Cohesion

Importance

10 Methods

How to fix Complexity

Complex Class

Duplication Side-by-Side

Filter issues like