SearchForm - Code Metrics - Inspection of "Update SearchForm" - silverstripe/silverstripe-cms - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#1758)

by Damian

created 2017-03-09 05:20 UTC

SearchForm A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	221
Duplicated Lines	8.14 %

Coupling/Cohesion

Components	1
Dependencies	9

Importance

Changes

Metric	Value
wmc	34
lcom	1
cbo	9
dl	18
loc	221
rs	9.2
c	0
b	0
f	0

8 Methods

Rating	Name	Duplication	Size	Complexity
B	__construct()	0	31	5
A	classesToSearch()	0	13	2
A	getClassesToSearch()	0	4	1
C	getResults()	18	68	17
B	addStarsToKeywords()	0	23	6
A	getSearchQuery()	0	4	1
A	setPageLength()	0	4	1
A	getPageLength()	0	4	1

How to fix Duplicated Code

<?php

namespace SilverStripe\CMS\Search;

use BadMethodCallException;
use SilverStripe\Assets\File;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Control\RequestHandler;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form;
use SilverStripe\Forms\FormAction;
use SilverStripe\Forms\HiddenField;
use SilverStripe\Forms\TextField;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\SS_List;
use Translatable;

/**
 * Standard basic search form which conducts a fulltext search on all {@link SiteTree}
 * objects.
 *
 * If multilingual content is enabled through the {@link Translatable} extension,
 * only pages the currently set language on the holder for this searchform are found.
 * The language is set through a hidden field in the form, which is prepoluated
 * with {@link Translatable::get_current_locale()} when then form is constructed.
 *
 * @see Use ModelController and SearchContext for a more generic search implementation based around DataObject
 */
class SearchForm extends Form
{
    /**
     * How many results are shown per page.
     * Relies on pagination being implemented in the search results template.
     *
     * @var int
     */
    protected $pageLength = 10;

    /**
     * Classes to search
     *
     * @var array
     */
    protected $classesToSearch = array(
        SiteTree::class,
        File::class
    );

    private static $casting = array(

        'SearchQuery' => 'Text'
    );

    /**
     * @skipUpgrade
     * @param RequestHandler $controller
     * @param string $name The name of the form (used in URL addressing)
     * @param FieldList $fields Optional, defaults to a single field named "Search". Search logic needs to be customized
     *  if fields are added to the form.
     * @param FieldList $actions Optional, defaults to a single field named "Go".
     */
    public function __construct(
        RequestHandler $controller = null,
        $name = 'SearchForm',
        FieldList $fields = null,
        FieldList $actions = null
    )
    {
        if (!$fields) {
            $fields = new FieldList(
                new TextField('Search', _t('SearchForm.SEARCH', 'Search'))
            );
        }

        if (class_exists('Translatable')
            && SiteTree::singleton()->hasExtension('Translatable')
        ) {
            $fields->push(new HiddenField('searchlocale', 'searchlocale', Translatable::get_current_locale()));
        }

        if (!$actions) {
            $actions = new FieldList(
                new FormAction("results", _t('SearchForm.GO', 'Go'))
            );
        }

        parent::__construct($controller, $name, $fields, $actions);
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

        $this->setFormMethod('get');

        $this->disableSecurityToken();
    }

    /**
     * Set the classes to search.
     * Currently you can only choose from "SiteTree" and "File", but a future version might improve this.
     *
     * @param array $classes
     */
    public function classesToSearch($classes)
    {
        $supportedClasses = array(SiteTree::class, File::class);
        $illegalClasses = array_diff($classes, $supportedClasses);
        if ($illegalClasses) {

            throw new BadMethodCallException(
                "SearchForm::classesToSearch() passed illegal classes '" . implode("', '", $illegalClasses)
                . "'.  At this stage, only File and SiteTree are allowed"
            );
        }
        $legalClasses = array_intersect($classes, $supportedClasses);
        $this->classesToSearch = $legalClasses;
    }

    /**
     * Get the classes to search
     *
     * @return array
     */
    public function getClassesToSearch()
    {
        return $this->classesToSearch;
    }

    /**
     * Return dataObjectSet of the results using current request to get info from form.
     * Wraps around {@link searchEngine()}.
     *
     * @return SS_List
     */
    public function getResults()
    {
        // Get request data from request handler
        $request = $this->getRequestHandler()->getRequest();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }

        // set language (if present)
        $locale = null;
        $origLocale = null;
        if (class_exists('Translatable')) {
            $locale = $request->requestVar('searchlocale');
            if (SiteTree::singleton()->hasExtension('Translatable') && $locale) {

                if ($locale === "ALL") {
                    Translatable::disable_locale_filter();
                } else {
                    $origLocale = Translatable::get_current_locale();

                    Translatable::set_current_locale($locale);
                }
            }
        }

        $keywords = $request->requestVar('Search');

        $andProcessor = create_function('$matches', '
	 		return " +" . $matches[2] . " +" . $matches[4] . " ";
	 	');
        $notProcessor = create_function('$matches', '
	 		return " -" . $matches[3];
	 	');

        $keywords = preg_replace_callback('/()("[^()"]+")( and )("[^"()]+")()/i', $andProcessor, $keywords);
        $keywords = preg_replace_callback('/(^| )([^() ]+)( and )([^ ()]+)( |$)/i', $andProcessor, $keywords);
        $keywords = preg_replace_callback('/(^| )(not )("[^"()]+")/i', $notProcessor, $keywords);
        $keywords = preg_replace_callback('/(^| )(not )([^() ]+)( |$)/i', $notProcessor, $keywords);

        $keywords = $this->addStarsToKeywords($keywords);

        $pageLength = $this->getPageLength();
        $start = $request->requestVar('start') ?: 0;

        if (strpos($keywords, '"') !== false || strpos($keywords, '+') !== false || strpos($keywords, '-') !== false || strpos($keywords, '*') !== false) {
            $results = DB::get_conn()->searchEngine($this->classesToSearch, $keywords, $start, $pageLength, "\"Relevance\" DESC", "", true);
        } else {
            $results = DB::get_conn()->searchEngine($this->classesToSearch, $keywords, $start, $pageLength);
        }

        // filter by permission
        if ($results) {
            foreach ($results as $result) {
                if (!$result->canView()) {
                    $results->remove($result);
                }
            }
        }

        // reset locale
        if (class_exists('Translatable')) {

            if (SiteTree::singleton()->hasExtension('Translatable') && $locale) {
                if ($locale == "ALL") {
                    Translatable::enable_locale_filter();
                } else {
                    Translatable::set_current_locale($origLocale);
                }
            }
        }

        return $results;
    }

    protected function addStarsToKeywords($keywords)
    {
        if (!trim($keywords)) {
            return "";
        }
        // Add * to each keyword
        $splitWords = preg_split("/ +/", trim($keywords));
        $newWords = [];
        while (list($i,$word) = each($splitWords)) {
<?php

function returnThreeValues() {
    return array('a', 'b', 'c');
}

list($a, $b, $c) = returnThreeValues();

print $a . " - " . $c;
            if ($word[0] == '"') {
                while (list($i,$subword) = each($splitWords)) {
<?php

function returnThreeValues() {
    return array('a', 'b', 'c');
}

list($a, $b, $c) = returnThreeValues();

print $a . " - " . $c;
                    $word .= ' ' . $subword;
                    if (substr($subword, -1) == '"') {
                        break;
                    }
                }
            } else {
                $word .= '*';
            }
            $newWords[] = $word;
        }
        return implode(" ", $newWords);
    }

    /**
     * Get the search query for display in a "You searched for ..." sentence.
     *
     * @return string
     */
    public function getSearchQuery()
    {
        return $this->getRequestHandler()->getRequest()->requestVar('Search');
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
    }

    /**
     * Set the maximum number of records shown on each page.
     *
     * @param int $length
     */
    public function setPageLength($length)
    {
        $this->pageLength = $length;
    }

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


1		<?php
2
3		namespace SilverStripe\CMS\Search;
4
5		use BadMethodCallException;
6		use SilverStripe\Assets\File;
7		use SilverStripe\CMS\Model\SiteTree;
8		use SilverStripe\Control\RequestHandler;
9		use SilverStripe\Forms\FieldList;
10		use SilverStripe\Forms\Form;
11		use SilverStripe\Forms\FormAction;
12		use SilverStripe\Forms\HiddenField;
13		use SilverStripe\Forms\TextField;
14		use SilverStripe\ORM\DB;
15		use SilverStripe\ORM\SS_List;
16		use Translatable;
17
18		/**
19		* Standard basic search form which conducts a fulltext search on all {@link SiteTree}
20		* objects.
21		*
22		* If multilingual content is enabled through the {@link Translatable} extension,
23		* only pages the currently set language on the holder for this searchform are found.
24		* The language is set through a hidden field in the form, which is prepoluated
25		* with {@link Translatable::get_current_locale()} when then form is constructed.
26		*
27		* @see Use ModelController and SearchContext for a more generic search implementation based around DataObject
28		*/
29		class SearchForm extends Form
30		{
31		/**
32		* How many results are shown per page.
33		* Relies on pagination being implemented in the search results template.
34		*
35		* @var int
36		*/
37		protected $pageLength = 10;
38
39		/**
40		* Classes to search
41		*
42		* @var array
43		*/
44		protected $classesToSearch = array(
45		SiteTree::class,
46		File::class
47		);
48
49		private static $casting = array(
		0 ignored issues – show Comprehensibility introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report Consider using a different property name as you override a private property of the parent class. Loading history...
50		'SearchQuery' => 'Text'
51		);
52
53		/**
54		* @skipUpgrade
55		* @param RequestHandler $controller
56		* @param string $name The name of the form (used in URL addressing)
57		* @param FieldList $fields Optional, defaults to a single field named "Search". Search logic needs to be customized
58		* if fields are added to the form.
59		* @param FieldList $actions Optional, defaults to a single field named "Go".
60		*/
61		public function __construct(
62		RequestHandler $controller = null,
63		$name = 'SearchForm',
64		FieldList $fields = null,
65		FieldList $actions = null
66		)
67		{
68		if (!$fields) {
69		$fields = new FieldList(
70		new TextField('Search', _t('SearchForm.SEARCH', 'Search'))
71		);
72		}
73
74		if (class_exists('Translatable')
75		&& SiteTree::singleton()->hasExtension('Translatable')
76		) {
77		$fields->push(new HiddenField('searchlocale', 'searchlocale', Translatable::get_current_locale()));
78		}
79
80		if (!$actions) {
81		$actions = new FieldList(
82		new FormAction("results", _t('SearchForm.GO', 'Go'))
83		);
84		}
85
86		parent::__construct($controller, $name, $fields, $actions);
		0 ignored issues – show Documentation introduced 2017-03-09 05:29 UTC by Report Bug Copy Issue Report `$controller` is of type `null\|object<SilverStripe\Control\RequestHandler>`, but the function expects a `object<SilverStripe\Control\Controller>`. 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...
87
88		$this->setFormMethod('get');
89
90		$this->disableSecurityToken();
91		}
92
93		/**
94		* Set the classes to search.
95		* Currently you can only choose from "SiteTree" and "File", but a future version might improve this.
96		*
97		* @param array $classes
98		*/
99		public function classesToSearch($classes)
100		{
101		$supportedClasses = array(SiteTree::class, File::class);
102		$illegalClasses = array_diff($classes, $supportedClasses);
103		if ($illegalClasses) {
		0 ignored issues – show Bug Best Practice introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report The expression `$illegalClasses` 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...
104		throw new BadMethodCallException(
105		"SearchForm::classesToSearch() passed illegal classes '" . implode("', '", $illegalClasses)
106		. "'. At this stage, only File and SiteTree are allowed"
107		);
108		}
109		$legalClasses = array_intersect($classes, $supportedClasses);
110		$this->classesToSearch = $legalClasses;
111		}
112
113		/**
114		* Get the classes to search
115		*
116		* @return array
117		*/
118		public function getClassesToSearch()
119		{
120		return $this->classesToSearch;
121		}
122
123		/**
124		* Return dataObjectSet of the results using current request to get info from form.
125		* Wraps around {@link searchEngine()}.
126		*
127		* @return SS_List
128		*/
129		public function getResults()
130		{
131		// Get request data from request handler
132		$request = $this->getRequestHandler()->getRequest();
		0 ignored issues – show Documentation Bug introduced 2017-03-09 05:29 UTC by Report Bug Copy Issue Report The method `getRequestHandler` does not exist on `object<SilverStripe\CMS\Search\SearchForm>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
133
134		// set language (if present)
135		$locale = null;
136		$origLocale = null;
137		if (class_exists('Translatable')) {
138		$locale = $request->requestVar('searchlocale');
139	View Code Duplication	if (SiteTree::singleton()->hasExtension('Translatable') && $locale) {
		0 ignored issues – show Duplication introduced 2017-03-09 05:29 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...
140		if ($locale === "ALL") {
141		Translatable::disable_locale_filter();
142		} else {
143		$origLocale = Translatable::get_current_locale();
144
145		Translatable::set_current_locale($locale);
146		}
147		}
148		}
149
150		$keywords = $request->requestVar('Search');
151
152		$andProcessor = create_function('$matches', '
153		return " +" . $matches[2] . " +" . $matches[4] . " ";
154		');
155		$notProcessor = create_function('$matches', '
156		return " -" . $matches[3];
157		');
158
159		$keywords = preg_replace_callback('/()("[^()"]+")( and )("[^"()]+")()/i', $andProcessor, $keywords);
160		$keywords = preg_replace_callback('/(^\| )([^() ]+)( and )([^ ()]+)( \|$)/i', $andProcessor, $keywords);
161		$keywords = preg_replace_callback('/(^\| )(not )("[^"()]+")/i', $notProcessor, $keywords);
162		$keywords = preg_replace_callback('/(^\| )(not )([^() ]+)( \|$)/i', $notProcessor, $keywords);
163
164		$keywords = $this->addStarsToKeywords($keywords);
165
166		$pageLength = $this->getPageLength();
167		$start = $request->requestVar('start') ?: 0;
168
169		if (strpos($keywords, '"') !== false \|\| strpos($keywords, '+') !== false \|\| strpos($keywords, '-') !== false \|\| strpos($keywords, '*') !== false) {
170		$results = DB::get_conn()->searchEngine($this->classesToSearch, $keywords, $start, $pageLength, "\"Relevance\" DESC", "", true);
171		} else {
172		$results = DB::get_conn()->searchEngine($this->classesToSearch, $keywords, $start, $pageLength);
173		}
174
175		// filter by permission
176		if ($results) {
177		foreach ($results as $result) {
178		if (!$result->canView()) {
179		$results->remove($result);
180		}
181		}
182		}
183
184		// reset locale
185	View Code Duplication	if (class_exists('Translatable')) {
		0 ignored issues – show Duplication introduced 2016-02-18 21:37 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...
186		if (SiteTree::singleton()->hasExtension('Translatable') && $locale) {
187		if ($locale == "ALL") {
188		Translatable::enable_locale_filter();
189		} else {
190		Translatable::set_current_locale($origLocale);
191		}
192		}
193		}
194
195		return $results;
196		}
197
198		protected function addStarsToKeywords($keywords)
199		{
200		if (!trim($keywords)) {
201		return "";
202		}
203		// Add * to each keyword
204		$splitWords = preg_split("/ +/", trim($keywords));
205		$newWords = [];
206		while (list($i,$word) = each($splitWords)) {
		0 ignored issues – show Unused Code introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report The assignment to `$i` is unused. Consider omitting it like so `list($first,,$third)`. This checks looks for assignemnts to variables using the `list(...)` function, where not all assigned variables are subsequently used. Consider the following code example. <?php function returnThreeValues() { return array('a', 'b', 'c'); } list($a, $b, $c) = returnThreeValues(); print $a . " - " . $c; Only the variables `$a` and `$c` are used. There was no need to assign `$b`. Instead, the list call could have been. list($a,, $c) = returnThreeValues(); Loading history...
207		if ($word[0] == '"') {
208		while (list($i,$subword) = each($splitWords)) {
		0 ignored issues – show Unused Code introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report The assignment to `$i` is unused. Consider omitting it like so `list($first,,$third)`. This checks looks for assignemnts to variables using the `list(...)` function, where not all assigned variables are subsequently used. Consider the following code example. <?php function returnThreeValues() { return array('a', 'b', 'c'); } list($a, $b, $c) = returnThreeValues(); print $a . " - " . $c; Only the variables `$a` and `$c` are used. There was no need to assign `$b`. Instead, the list call could have been. list($a,, $c) = returnThreeValues(); Loading history...
209		$word .= ' ' . $subword;
210		if (substr($subword, -1) == '"') {
211		break;
212		}
213		}
214		} else {
215		$word .= '*';
216		}
217		$newWords[] = $word;
218		}
219		return implode(" ", $newWords);
220		}
221
222		/**
223		* Get the search query for display in a "You searched for ..." sentence.
224		*
225		* @return string
226		*/
227		public function getSearchQuery()
228		{
229		return $this->getRequestHandler()->getRequest()->requestVar('Search');
		0 ignored issues – show Documentation Bug introduced 2017-03-09 05:29 UTC by Report Bug Copy Issue Report The method `getRequestHandler` does not exist on `object<SilverStripe\CMS\Search\SearchForm>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
230		}
231
232		/**
233		* Set the maximum number of records shown on each page.
234		*
235		* @param int $length
236		*/
237		public function setPageLength($length)
238		{
239		$this->pageLength = $length;
240		}
241
242		/**
243		* @return int
244		*/
245		public function getPageLength()
246		{
247		return $this->pageLength;
248		}
249		}
250

silverstripe / silverstripe-cms

Pull Request — master (#1758)

SearchForm A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

8 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like