TagField::saveInto() - Code Metrics - Inspection of "Removed is_numeric check in SaveInto function" - silverstripe/silverstripe-tagfield - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#71)

by Myles

created 2016-02-03 23:08 UTC

TagField::saveInto() C

↳ Parent: TagField

Complexity

Conditions	8
Paths	10

Size

Total Lines	42
Code Lines	21

Duplication

Lines	0
Ratio	0 %

Metric	Value
dl	0
loc	42
rs	5.3846
cc	8
eloc	21
nc	10
nop	1

<?php

/**
 * Provides a tagging interface, storing links between tag DataObjects and a parent DataObject.
 *
 * @package forms
 * @subpackage fields
 */
class TagField extends DropdownField
namespace YourVendor;

class YourClass { }
{
    /**
     * @var array
     */
    public static $allowed_actions = array(
        'suggest',
    );

    /**
     * @var bool
     */
    protected $shouldLazyLoad = false;

    /**
     * @var int
     */
    protected $lazyLoadItemLimit = 10;

    /**
     * @var bool
     */
    protected $canCreate = true;

    /**
     * @var string
     */
    protected $titleField = 'Title';

    /**
     * @var bool
     */
    protected $isMultiple = true;

    /**
     * @param string $name
     * @param string $title
     * @param null|DataList $source
     * @param null|DataList $value
     */
    public function __construct($name, $title = '', $source = null, $value = null)
    {
        parent::__construct($name, $title, $source, $value);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}
    }

    /**
     * @return bool
     */
    public function getShouldLazyLoad()
    {
        return $this->shouldLazyLoad;
    }

    /**
     * @param bool $shouldLazyLoad
     *
     * @return static
     */
    public function setShouldLazyLoad($shouldLazyLoad)
    {
        $this->shouldLazyLoad = $shouldLazyLoad;

        return $this;
    }

    /**
     * @return int
     */
    public function getLazyLoadItemLimit()
    {
        return $this->lazyLoadItemLimit;
    }

    /**
     * @param int $lazyLoadItemLimit
     *
     * @return static
     */
    public function setLazyLoadItemLimit($lazyLoadItemLimit)
    {
        $this->lazyLoadItemLimit = $lazyLoadItemLimit;

        return $this;
    }

    /**
     * @return bool
     */
    public function getIsMultiple()
    {
        return $this->isMultiple;
    }

    /**
     * @param bool $isMultiple
     *
     * @return static
     */
    public function setIsMultiple($isMultiple)
    {
        $this->isMultiple = $isMultiple;

        return $this;
    }

    /**
     * @return bool
     */
    public function getCanCreate()
    {
        return $this->canCreate;
    }

    /**
     * @param bool $canCreate
     *
     * @return static
     */
    public function setCanCreate($canCreate)
    {
        $this->canCreate = $canCreate;

        return $this;
    }

    /**
     * @return string
     */
    public function getTitleField()
    {
        return $this->titleField;
    }

    /**
     * @param string $titleField
     *
     * @return $this
     */
    public function setTitleField($titleField)
    {
        $this->titleField = $titleField;

        return $this;
    }

    /**
     * {@inheritdoc}
     */
    public function Field($properties = array())

    {
        Requirements::css(TAG_FIELD_DIR . '/css/select2.min.css');
        Requirements::css(TAG_FIELD_DIR . '/css/TagField.css');

        Requirements::javascript(THIRDPARTY_DIR . '/jquery/jquery.js');
        Requirements::javascript(THIRDPARTY_DIR . '/jquery-entwine/dist/jquery.entwine-dist.js');
        Requirements::javascript(TAG_FIELD_DIR . '/js/select2.js');
        Requirements::javascript(TAG_FIELD_DIR . '/js/TagField.js');

        $this->addExtraClass('ss-tag-field');

        if ($this->getIsMultiple()) {
            $this->setAttribute('multiple', 'multiple');
        }

        if ($this->shouldLazyLoad) {
            $this->setAttribute('data-ss-tag-field-suggest-url', $this->getSuggestURL());
        } else {
            $properties = array_merge($properties, array(
                'Options' => $this->getOptions()
            ));
        }

        return $this
            ->customise($properties)
            ->renderWith(array("templates/TagField"));
    }

    /**
     * @return string
     */
    protected function getSuggestURL()
    {
        return Controller::join_links($this->Link(), 'suggest');
    }

    /**
     * @return ArrayList
     */
    protected function getOptions()
    {
        $options = ArrayList::create();

        $source = $this->getSource();

        if(!$source) {
            $source = new ArrayList();
        }

        $dataClass = $source->dataClass();

        $values = $this->Value();

        if(!$values) {
            return $options;
        }

        if(is_array($values)) {
            $values = DataList::create($dataClass)->filter('Title', $values);
        }

        $ids = $values->column('Title');

        $titleField = $this->getTitleField();

        foreach($source as $object) {
            $options->push(
            ArrayData::create(array(
                'Title' => $object->$titleField,
                'Value' => $object->Title,
                'Selected' => in_array($object->Title, $ids),
                ))
            );
        }

        return $options;
 	}

    /**
     * {@inheritdoc}
     */
    public function setValue($value, $source = null)
    {
        if ($source instanceof DataObject) {
            $name = $this->getName();

            if ($source->hasMethod($name)) {
                $value = $source->$name()->column('Title');
            }
        } elseif ($value instanceof SS_List) {
            $value = $value->column('Title');
        }

        if (!is_array($value)) {
            return parent::setValue($value);
        }

        return parent::setValue(array_filter($value));
    }

    /**
     * {@inheritdoc}
     */
    public function getAttributes()
    {
        return array_merge(
            parent::getAttributes(),
            array('name' => $this->getName() . '[]')
        );
    }

    /**
     * {@inheritdoc}
     */
    public function saveInto(DataObjectInterface $record)
    {
        parent::saveInto($record);

        $name = $this->getName();

        $titleField = $this->getTitleField();

        $source = $this->getSource();

        $values = $this->Value();

        $relation = $record->$name();

        $ids = array();

        if(!$values) {
            $values = array();
        }

        if(empty($record) || empty($source) || empty($titleField)) {
            return;
        }

        if(!$record->hasMethod($name)) {
            throw new Exception(
                sprintf("%s does not have a %s method", get_class($record), $name)
            );
        }

        foreach ($values as $key => $value) {
            // Get or create record
            $record = $this->getOrCreateTag($value);
            if($record) {
                $ids[] = $record->ID;
                $values[$key] = $record->Title;
            }
        }

        $relation->setByIDList(array_filter($ids));

 	}

    /**
     * Get or create tag with the given value
     *
     * @param string $term
     * @return DataObject
     */
    protected function getOrCreateTag($term)
    {
        // Check if existing record can be found
		$source = $this->getSource();
		$titleField = $this->getTitleField();
		$record = $source
			->filter($titleField, $term)
			->first();
		if($record) {
			return $record;
		}

		// Create new instance if not yet saved
		if ($this->getCanCreate()) {
			$dataClass = $source->dataClass();
			$record = Injector::inst()->create($dataClass);
			$record->{$titleField} = $term;
			$record->write();
			return $record;
		} else {
			return false;
class Author {
    private $name;

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

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
		}
    }

    /**
     * Returns a JSON string of tags, for lazy loading.
     *
     * @param SS_HTTPRequest $request
     *
     * @return SS_HTTPResponse
     */
    public function suggest(SS_HTTPRequest $request)
    {
        $tags = $this->getTags($request->getVar('term'));

        $response = new SS_HTTPResponse();
        $response->addHeader('Content-Type', 'application/json');
        $response->setBody(json_encode(array('items' => $tags)));

        return $response;
    }

    /**
     * Returns array of arrays representing tags.
     *
     * @param string $term
     *
     * @return array
     */
    protected function getTags($term)
    {
        /**
         * @var DataList $source
         */
        $source = $this->getSource();

        $titleField = $this->getTitleField();

        $query = $source
            ->filter($titleField . ':PartialMatch:nocase', $term)
            ->sort($titleField)
            ->limit($this->getLazyLoadItemLimit());

        // Map into a distinct list
        $items = array();
        $titleField = $this->getTitleField();
        foreach ($query->map('ID', $titleField) as $id => $title) {
            $items[$title] = array(
                'id' => $title,
                'text' => $title
            );
        }

        return array_values($items);
    }

    /**
     * DropdownField assumes value will be a scalar so we must
     * override validate. This only applies to Silverstripe 3.2+
     *
     * @param Validator $validator
     * @return bool
     */
    public function validate($validator)
    {
        return true;
    }
}


1		<?php
2
3		/**
4		* Provides a tagging interface, storing links between tag DataObjects and a parent DataObject.
5		*
6		* @package forms
7		* @subpackage fields
8		*/
9		class TagField extends DropdownField
		0 ignored issues – show Coding Style Compatibility introduced 2015-11-20 23:54 UTC by Report Bug Copy Issue Report PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
10		{
11		/**
12		* @var array
13		*/
14		public static $allowed_actions = array(
15		'suggest',
16		);
17
18		/**
19		* @var bool
20		*/
21		protected $shouldLazyLoad = false;
22
23		/**
24		* @var int
25		*/
26		protected $lazyLoadItemLimit = 10;
27
28		/**
29		* @var bool
30		*/
31		protected $canCreate = true;
32
33		/**
34		* @var string
35		*/
36		protected $titleField = 'Title';
37
38		/**
39		* @var bool
40		*/
41		protected $isMultiple = true;
42
43		/**
44		* @param string $name
45		* @param string $title
46		* @param null\|DataList $source
47		* @param null\|DataList $value
48		*/
49		public function __construct($name, $title = '', $source = null, $value = null)
50		{
51		parent::__construct($name, $title, $source, $value);
		0 ignored issues – show Bug introduced 2015-11-20 23:54 UTC by Report Bug Copy Issue Report It seems like `$source` defined by parameter `$source` on line `49` can be `null`; however, `DropdownField::__construct()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history... Documentation introduced 2015-11-20 23:54 UTC by Report Bug Copy Issue Report `$value` is of type `null\|object<DataList>`, but the function expects a `string`. 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...
52		}
53
54		/**
55		* @return bool
56		*/
57		public function getShouldLazyLoad()
58		{
59		return $this->shouldLazyLoad;
60		}
61
62		/**
63		* @param bool $shouldLazyLoad
64		*
65		* @return static
66		*/
67		public function setShouldLazyLoad($shouldLazyLoad)
68		{
69		$this->shouldLazyLoad = $shouldLazyLoad;
70
71		return $this;
72		}
73
74		/**
75		* @return int
76		*/
77		public function getLazyLoadItemLimit()
78		{
79		return $this->lazyLoadItemLimit;
80		}
81
82		/**
83		* @param int $lazyLoadItemLimit
84		*
85		* @return static
86		*/
87		public function setLazyLoadItemLimit($lazyLoadItemLimit)
88		{
89		$this->lazyLoadItemLimit = $lazyLoadItemLimit;
90
91		return $this;
92		}
93
94		/**
95		* @return bool
96		*/
97		public function getIsMultiple()
98		{
99		return $this->isMultiple;
100		}
101
102		/**
103		* @param bool $isMultiple
104		*
105		* @return static
106		*/
107		public function setIsMultiple($isMultiple)
108		{
109		$this->isMultiple = $isMultiple;
110
111		return $this;
112		}
113
114		/**
115		* @return bool
116		*/
117		public function getCanCreate()
118		{
119		return $this->canCreate;
120		}
121
122		/**
123		* @param bool $canCreate
124		*
125		* @return static
126		*/
127		public function setCanCreate($canCreate)
128		{
129		$this->canCreate = $canCreate;
130
131		return $this;
132		}
133
134		/**
135		* @return string
136		*/
137		public function getTitleField()
138		{
139		return $this->titleField;
140		}
141
142		/**
143		* @param string $titleField
144		*
145		* @return $this
146		*/
147		public function setTitleField($titleField)
148		{
149		$this->titleField = $titleField;
150
151		return $this;
152		}
153
154		/**
155		* {@inheritdoc}
156		*/
157	View Code Duplication	public function Field($properties = array())
		0 ignored issues – show Duplication introduced 2015-11-20 23:54 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
158		{
159		Requirements::css(TAG_FIELD_DIR . '/css/select2.min.css');
160		Requirements::css(TAG_FIELD_DIR . '/css/TagField.css');
161
162		Requirements::javascript(THIRDPARTY_DIR . '/jquery/jquery.js');
163		Requirements::javascript(THIRDPARTY_DIR . '/jquery-entwine/dist/jquery.entwine-dist.js');
164		Requirements::javascript(TAG_FIELD_DIR . '/js/select2.js');
165		Requirements::javascript(TAG_FIELD_DIR . '/js/TagField.js');
166
167		$this->addExtraClass('ss-tag-field');
168
169		if ($this->getIsMultiple()) {
170		$this->setAttribute('multiple', 'multiple');
171		}
172
173		if ($this->shouldLazyLoad) {
174		$this->setAttribute('data-ss-tag-field-suggest-url', $this->getSuggestURL());
175		} else {
176		$properties = array_merge($properties, array(
177		'Options' => $this->getOptions()
178		));
179		}
180
181		return $this
182		->customise($properties)
183		->renderWith(array("templates/TagField"));
184		}
185
186		/**
187		* @return string
188		*/
189		protected function getSuggestURL()
190		{
191		return Controller::join_links($this->Link(), 'suggest');
192		}
193
194		/**
195		* @return ArrayList
196		*/
197		protected function getOptions()
198		{
199		$options = ArrayList::create();
200
201		$source = $this->getSource();
202
203		if(!$source) {
204		$source = new ArrayList();
205		}
206
207		$dataClass = $source->dataClass();
208
209		$values = $this->Value();
210
211		if(!$values) {
212		return $options;
213		}
214
215		if(is_array($values)) {
216		$values = DataList::create($dataClass)->filter('Title', $values);
217		}
218
219		$ids = $values->column('Title');
220
221		$titleField = $this->getTitleField();
222
223		foreach($source as $object) {
224		$options->push(
225		ArrayData::create(array(
226		'Title' => $object->$titleField,
227		'Value' => $object->Title,
228		'Selected' => in_array($object->Title, $ids),
229		))
230		);
231		}
232
233		return $options;
234		}
235
236		/**
237		* {@inheritdoc}
238		*/
239		public function setValue($value, $source = null)
240		{
241		if ($source instanceof DataObject) {
242		$name = $this->getName();
243
244		if ($source->hasMethod($name)) {
245		$value = $source->$name()->column('Title');
246		}
247		} elseif ($value instanceof SS_List) {
248		$value = $value->column('Title');
249		}
250
251		if (!is_array($value)) {
252		return parent::setValue($value);
253		}
254
255		return parent::setValue(array_filter($value));
256		}
257
258		/**
259		* {@inheritdoc}
260		*/
261		public function getAttributes()
262		{
263		return array_merge(
264		parent::getAttributes(),
265		array('name' => $this->getName() . '[]')
266		);
267		}
268
269		/**
270		* {@inheritdoc}
271		*/
272		public function saveInto(DataObjectInterface $record)
273		{
274		parent::saveInto($record);
275
276		$name = $this->getName();
277
278		$titleField = $this->getTitleField();
279
280		$source = $this->getSource();
281
282		$values = $this->Value();
283
284		$relation = $record->$name();
285
286		$ids = array();
287
288		if(!$values) {
289		$values = array();
290		}
291
292		if(empty($record) \|\| empty($source) \|\| empty($titleField)) {
293		return;
294		}
295
296		if(!$record->hasMethod($name)) {
297		throw new Exception(
298		sprintf("%s does not have a %s method", get_class($record), $name)
299		);
300		}
301
302		foreach ($values as $key => $value) {
303		// Get or create record
304		$record = $this->getOrCreateTag($value);
305		if($record) {
306		$ids[] = $record->ID;
307		$values[$key] = $record->Title;
308		}
309		}
310
311		$relation->setByIDList(array_filter($ids));
312
313		}
314
315		/**
316		* Get or create tag with the given value
317		*
318		* @param string $term
319		* @return DataObject
320		*/
321		protected function getOrCreateTag($term)
322		{
323		// Check if existing record can be found
324		$source = $this->getSource();
325		$titleField = $this->getTitleField();
326		$record = $source
327		->filter($titleField, $term)
328		->first();
329		if($record) {
330		return $record;
331		}
332
333		// Create new instance if not yet saved
334		if ($this->getCanCreate()) {
335		$dataClass = $source->dataClass();
336		$record = Injector::inst()->create($dataClass);
337		$record->{$titleField} = $term;
338		$record->write();
339		return $record;
340		} else {
341		return false;
		0 ignored issues – show Bug Best Practice introduced 2016-02-03 23:11 UTC by Report Bug Copy Issue Report The return type of `return false;` (`false`) is incompatible with the return type documented by `TagField::getOrCreateTag` of type `DataObject`. If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author { private $name; public function __construct($name) { $this->name = $name; } public function getName() { return $this->name; } } abstract class Post { public function getAuthor() { return 'Johannes'; } } class BlogPost extends Post { public function getAuthor() { return new Author('Johannes'); } } class ForumPost extends Post { /* ... */ } function my_function(Post $post) { echo strtoupper($post->getAuthor()); } Our function `my_function` expects a `Post` object, and outputs the author of the post. The base class `Post` returns a simple string and outputting a simple string will work just fine. However, the child class `BlogPost` which is a sub-type of `Post` instead decided to return an `object`, and is therefore violating the SOLID principles. If a `BlogPost` were passed to `my_function`, PHP would not complain, but ultimately fail when executing the `strtoupper` call in its body. Loading history...
342		}
343		}
344
345		/**
346		* Returns a JSON string of tags, for lazy loading.
347		*
348		* @param SS_HTTPRequest $request
349		*
350		* @return SS_HTTPResponse
351		*/
352		public function suggest(SS_HTTPRequest $request)
353		{
354		$tags = $this->getTags($request->getVar('term'));
355
356		$response = new SS_HTTPResponse();
357		$response->addHeader('Content-Type', 'application/json');
358		$response->setBody(json_encode(array('items' => $tags)));
359
360		return $response;
361		}
362
363		/**
364		* Returns array of arrays representing tags.
365		*
366		* @param string $term
367		*
368		* @return array
369		*/
370		protected function getTags($term)
371		{
372		/**
373		* @var DataList $source
374		*/
375		$source = $this->getSource();
376
377		$titleField = $this->getTitleField();
378
379		$query = $source
380		->filter($titleField . ':PartialMatch:nocase', $term)
381		->sort($titleField)
382		->limit($this->getLazyLoadItemLimit());
383
384		// Map into a distinct list
385		$items = array();
386		$titleField = $this->getTitleField();
387		foreach ($query->map('ID', $titleField) as $id => $title) {
388		$items[$title] = array(
389		'id' => $title,
390		'text' => $title
391		);
392		}
393
394		return array_values($items);
395		}
396
397		/**
398		* DropdownField assumes value will be a scalar so we must
399		* override validate. This only applies to Silverstripe 3.2+
400		*
401		* @param Validator $validator
402		* @return bool
403		*/
404		public function validate($validator)
405		{
406		return true;
407		}
408		}
409

silverstripe / silverstripe-tagfield

Pull Request — master (#71)

TagField::saveInto() C

Complexity

Size

Duplication

Duplication Side-by-Side

Filter issues like