TagField - Code Metrics - Inspection of "Update changelog for 1.3.0" - silverstripe/silverstripe-tagfield - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( d80d0a...d3563c )

by Damian

created 2016-05-19 00:33 UTC

TagField B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	413
Duplicated Lines	6.78 %

Coupling/Cohesion

Components	1
Dependencies	12

Importance

Changes	12
Bugs	4	Features	3

Metric	Value
wmc	43
c	12
b	4
f	3
lcom	1
cbo	12
dl	28
loc	413
rs	8.3157

22 Methods

Rating	Name	Duplication	Size	Complexity
A	performReadonlyTransformation()	0	6	1
A	__construct()	0	4	1
A	getShouldLazyLoad()	0	4	1
A	setShouldLazyLoad()	0	6	1
A	getLazyLoadItemLimit()	0	4	1
A	setLazyLoadItemLimit()	0	6	1
A	getIsMultiple()	0	4	1
A	setIsMultiple()	0	6	1
A	getCanCreate()	0	4	1
A	setCanCreate()	0	6	1
A	getTitleField()	0	4	1
A	setTitleField()	0	6	1
B	Field()	28	28	3
A	getSuggestURL()	0	4	1
B	getOptions()	0	39	5
B	setValue()	0	18	5
A	getAttributes()	0	7	1
D	saveInto()	0	46	10
A	getOrCreateTag()	0	19	2
A	suggest()	0	10	1
B	getTags()	0	26	2
A	validate()	0	4	1

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like TagField often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use TagField, and based on these observations, apply Extract Interface, too.

<?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 acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $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();
$myVar = 'Value';
$higher = false;

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

        // Mark selected tags while still returning a full list of possible options
        $ids = array(); // empty fallback array for comparing
        $values = $this->Value();
        if($values){
            // @TODO conversion from array to DataList to array...(?)
            if(is_array($values)) {
                $values = DataList::create($dataClass)->filter('ID', $values);
            }
            $ids = $values->column('ID');
        }

        $titleField = $this->getTitleField();

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

        return $options;
    }

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

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

        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();

        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)
            );
        }

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

        foreach ($values as $i => $value) {
            if (!is_numeric($value)) {
                if (!$this->getCanCreate()) {
                    unset($values[$i]);
                    continue;
                }

                // Get or create record
                $record = $this->getOrCreateTag($value);
                $values[$i] = $record->ID;
            }
        }

        if ($values instanceof SS_List) {
            $values = iterator_to_array($values);
        }

        $relation->setByIDList(array_filter($values));
    }

    /**
     * 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
        $dataClass = $source->dataClass();
        $record = Injector::inst()->create($dataClass);
        $record->{$titleField} = $term;
        $record->write();
        return $record;
    }

    /**
     * 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' => $id,
                '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;
    }
    
    /**
     * Converts the field to a readonly variant.
     *
     * @return TagField_Readonly
     */
    public function performReadonlyTransformation()
    {
        $copy = $this->castedCopy('TagField_Readonly');
        $copy->setSource($this->getSource());
        return $copy;
    }
}

/**
 * A readonly extension of TagField useful for non-editable items within the CMS.
 *
 * @package forms
 * @subpackage fields
 */
class TagField_Readonly extends TagField
namespace YourVendor;

class YourClass { }
{
    protected $readonly = true;
    
    /**
     * Render the readonly field as HTML.
     *
     * @param array $properties
     * @return HTMLText
     */
    public function Field($properties = array())
    {
        $options = array();
        
        foreach ($this->getOptions()->filter('Selected', true) as $option) {
            $options[] = $option->Title;
        }
        
        $field = ReadonlyField::create($this->name.'_Readonly', $this->title);
        
        $field->setForm($this->form);
        $field->setValue(implode(', ', $options));
        return $field->Field();
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());
}
    }
}


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 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... Bug introduced 2016-02-10 19:46 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...
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();
		0 ignored issues – show Unused Code introduced 2015-12-31 05:50 UTC by Report Bug Copy Issue Report `$values` 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...
210
211		// Mark selected tags while still returning a full list of possible options
212		$ids = array(); // empty fallback array for comparing
213		$values = $this->Value();
214		if($values){
215		// @TODO conversion from array to DataList to array...(?)
216		if(is_array($values)) {
217		$values = DataList::create($dataClass)->filter('ID', $values);
218		}
219		$ids = $values->column('ID');
220		}
221
222		$titleField = $this->getTitleField();
223
224		foreach ($source as $object) {
225		$options->push(
226		ArrayData::create(array(
227		'Title' => $object->$titleField,
228		'Value' => $object->ID,
229		'Selected' => in_array($object->ID, $ids),
230		))
231		);
232		}
233
234		return $options;
235		}
236
237		/**
238		* {@inheritdoc}
239		*/
240		public function setValue($value, $source = null)
241		{
242		if ($source instanceof DataObject) {
243		$name = $this->getName();
244
245		if ($source->hasMethod($name)) {
246		$value = $source->$name()->getIDList();
247		}
248		} elseif ($value instanceof SS_List) {
249		$value = $value->column('ID');
250		}
251
252		if (!is_array($value)) {
253		return parent::setValue($value);
254		}
255
256		return parent::setValue(array_filter($value));
257		}
258
259		/**
260		* {@inheritdoc}
261		*/
262		public function getAttributes()
263		{
264		return array_merge(
265		parent::getAttributes(),
266		array('name' => $this->getName() . '[]')
267		);
268		}
269
270		/**
271		* {@inheritdoc}
272		*/
273		public function saveInto(DataObjectInterface $record)
274		{
275		parent::saveInto($record);
276
277		$name = $this->getName();
278		$titleField = $this->getTitleField();
279
280		$source = $this->getSource();
281
282		$values = $this->Value();
283
284		if (!$values) {
285		$values = array();
286		}
287
288		if (empty($record) \|\| empty($source) \|\| empty($titleField)) {
289		return;
290		}
291
292		if (!$record->hasMethod($name)) {
293		throw new Exception(
294		sprintf("%s does not have a %s method", get_class($record), $name)
295		);
296		}
297
298		$relation = $record->$name();
299
300		foreach ($values as $i => $value) {
301		if (!is_numeric($value)) {
302		if (!$this->getCanCreate()) {
303		unset($values[$i]);
304		continue;
305		}
306
307		// Get or create record
308		$record = $this->getOrCreateTag($value);
309		$values[$i] = $record->ID;
310		}
311		}
312
313		if ($values instanceof SS_List) {
314		$values = iterator_to_array($values);
315		}
316
317		$relation->setByIDList(array_filter($values));
318		}
319
320		/**
321		* Get or create tag with the given value
322		*
323		* @param string $term
324		* @return DataObject
325		*/
326		protected function getOrCreateTag($term)
327		{
328		// Check if existing record can be found
329		$source = $this->getSource();
330		$titleField = $this->getTitleField();
331		$record = $source
332		->filter($titleField, $term)
333		->first();
334		if ($record) {
335		return $record;
336		}
337
338		// Create new instance if not yet saved
339		$dataClass = $source->dataClass();
340		$record = Injector::inst()->create($dataClass);
341		$record->{$titleField} = $term;
342		$record->write();
343		return $record;
344		}
345
346		/**
347		* Returns a JSON string of tags, for lazy loading.
348		*
349		* @param SS_HTTPRequest $request
350		*
351		* @return SS_HTTPResponse
352		*/
353		public function suggest(SS_HTTPRequest $request)
354		{
355		$tags = $this->getTags($request->getVar('term'));
356
357		$response = new SS_HTTPResponse();
358		$response->addHeader('Content-Type', 'application/json');
359		$response->setBody(json_encode(array('items' => $tags)));
360
361		return $response;
362		}
363
364		/**
365		* Returns array of arrays representing tags.
366		*
367		* @param string $term
368		*
369		* @return array
370		*/
371		protected function getTags($term)
372		{
373		/**
374		* @var DataList $source
375		*/
376		$source = $this->getSource();
377
378		$titleField = $this->getTitleField();
379
380		$query = $source
381		->filter($titleField . ':PartialMatch:nocase', $term)
382		->sort($titleField)
383		->limit($this->getLazyLoadItemLimit());
384
385		// Map into a distinct list
386		$items = array();
387		$titleField = $this->getTitleField();
388		foreach ($query->map('ID', $titleField) as $id => $title) {
389		$items[$title] = array(
390		'id' => $id,
391		'text' => $title
392		);
393		}
394
395		return array_values($items);
396		}
397
398		/**
399		* DropdownField assumes value will be a scalar so we must
400		* override validate. This only applies to Silverstripe 3.2+
401		*
402		* @param Validator $validator
403		* @return bool
404		*/
405		public function validate($validator)
406		{
407		return true;
408		}
409
410		/**
411		* Converts the field to a readonly variant.
412		*
413		* @return TagField_Readonly
414		*/
415		public function performReadonlyTransformation()
416		{
417		$copy = $this->castedCopy('TagField_Readonly');
418		$copy->setSource($this->getSource());
419		return $copy;
420		}
421		}
422
423		/**
424		* A readonly extension of TagField useful for non-editable items within the CMS.
425		*
426		* @package forms
427		* @subpackage fields
428		*/
429		class TagField_Readonly extends TagField
		0 ignored issues – show Coding Style Compatibility introduced 2016-02-25 02:32 UTC by Report Bug Copy Issue Report PSR1 recommends that each class should be in its own file to aid autoloaders. Having each class in a dedicated file usually plays nice with PSR autoloaders and is therefore a well established practice. If you use other autoloaders, you might not want to follow this rule. Loading history... Coding Style Compatibility introduced 2016-02-25 02:32 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...
430		{
431		protected $readonly = true;
432
433		/**
434		* Render the readonly field as HTML.
435		*
436		* @param array $properties
437		* @return HTMLText
438		*/
439		public function Field($properties = array())
440		{
441		$options = array();
442
443		foreach ($this->getOptions()->filter('Selected', true) as $option) {
444		$options[] = $option->Title;
445		}
446
447		$field = ReadonlyField::create($this->name.'_Readonly', $this->title);
448
449		$field->setForm($this->form);
450		$field->setValue(implode(', ', $options));
451		return $field->Field();
		0 ignored issues – show Bug Best Practice introduced 2016-02-25 02:32 UTC by Report Bug Copy Issue Report The return type of `return $field->Field();` (`string`) is incompatible with the return type of the parent method `TagField::Field` of type `HTMLText`. 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...
452		}
453		}
454

silverstripe / silverstripe-tagfield