DeprecatedCore - Code Metrics - Inspection of "Merge pull request #725 from AgelxNash/dependency" - evolution-cms/evolution - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — develop ( cb7ecf...5e631f )

by Dmytro

created 2018-06-12 15:18 UTC

DeprecatedCore B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	313
Duplicated Lines	1.92 %

Coupling/Cohesion

Components	0
Dependencies	2

Importance

Changes

Metric	Value
dl	6
loc	313
rs	8.3396
c	0
b	0
f	0
wmc	44
lcom	0
cbo	2

16 Methods

Rating	Name	Duplication	Size	Complexity
A	dbConnect()	0	6	1
A	dbQuery()	0	6	1
A	recordCount()	0	6	1
A	fetchRow()	0	6	1
A	affectedRows()	0	6	1
A	insertId()	0	6	1
A	dbClose()	0	5	1
C	makeList()	0	28	8
A	getUserData()	0	8	1
B	insideManager()	0	16	7
A	putChunk()	0	6	1
A	getDocGroups()	0	6	1
A	changePassword()	0	6	1
B	userLoggedIn()	0	26	5
D	getFormVars()	0	33	9
A	webAlert()	6	17	4

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 DeprecatedCore 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 DeprecatedCore, and based on these observations, apply Extract Interface, too.

<?php namespace EvolutionCMS\Legacy;

use EvolutionCMS\Interfaces\DeprecatedCoreInterface;

class DeprecatedCore implements DeprecatedCoreInterface
{
    /**
    * @deprecated
    *
    * return @void
    */
    public function dbConnect()
    {
        $modx = evolutionCMS();
        $modx->getDatabase()->connect();
        $modx->rs = $modx->getDatabase()->conn;
    }

    /**
     * @deprecated
     *
     * @param $sql
     * @return bool|mysqli_result|resource

     */
    public function dbQuery($sql)
    {
        $modx = evolutionCMS();

        return $modx->getDatabase()->query($sql);
    }

    /**
     * @deprecated
     *
     * @param $rs
     * @return int
     */
    public function recordCount($rs)

    {
        $modx = evolutionCMS();

        return $modx->getDatabase()->getRecordCount($rs);
    }

    /**
     * @deprecated
     *
     * @param $rs
     * @param string $mode
     * @return array|bool|mixed|object|stdClass
     */
    public function fetchRow($rs, $mode = 'assoc')

    {
        $modx = evolutionCMS();

        return $modx->getDatabase()->getRow($rs, $mode);
    }

    /**
     * @deprecated
     *
     * @param $rs
     * @return int
     */
    public function affectedRows($rs)

    {
        $modx = evolutionCMS();

        return $modx->getDatabase()->getAffectedRows($rs);
    }

    /**
     * @deprecated
     *
     * @param $rs
     * @return int|mixed
     */
    public function insertId($rs)

    {
        $modx = evolutionCMS();

        return $modx->getDatabase()->getInsertId($rs);
    }

    /**
     * @deprecated
     *
     * @return void
     */
    public function dbClose()
    {
        $modx = evolutionCMS();
        $modx->getDatabase()->disconnect();
    }

    /**
     * @deprecated
     *
     * @param array $array
     * @param string $ulroot
     * @param string $ulprefix
     * @param string $type
     * @param bool $ordered
     * @param int $tablevel
     * @return string
     */
    public function makeList($array, $ulroot = 'root', $ulprefix = 'sub_', $type = '', $ordered = false, $tablevel = 0)
    {
        // first find out whether the value passed is an array
        if (!is_array($array)) {
            return "<ul><li>Bad list</li></ul>";
        }
        if (!empty ($type)) {
            $typestr = " style='list-style-type: $type'";
        } else {
            $typestr = "";
        }
        $tabs = "";
        for ($i = 0; $i < $tablevel; $i++) {
            $tabs .= "\t";
        }
        $listhtml = $ordered == true ? $tabs . "<ol class='$ulroot'$typestr>\n" : $tabs . "<ul class='$ulroot'$typestr>\n";

        foreach ($array as $key => $value) {
            if (is_array($value)) {
                $listhtml .= $tabs . "\t<li>" . $key . "\n" . $this->makeList($value, $ulprefix . $ulroot, $ulprefix,

                        $type, $ordered, $tablevel + 2) . $tabs . "\t</li>\n";
            } else {
                $listhtml .= $tabs . "\t<li>" . $value . "</li>\n";
            }
        }
        $listhtml .= $ordered == true ? $tabs . "</ol>\n" : $tabs . "</ul>\n";


        return $listhtml;
    }

    /**
     * @deprecated
     *
     * @return array
     */
    public function getUserData()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

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

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

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $client = array();
        $client['ip'] = $_SERVER['REMOTE_ADDR'];
        $client['ua'] = $_SERVER['HTTP_USER_AGENT'];

        return $client;
    }

    /**
     * Returns true, install or interact when inside manager
     *
     * @deprecated
     *
     * @return bool|string
     */
    public function insideManager()
    {
        $m = false;

        if (defined('IN_MANAGER_MODE') && IN_MANAGER_MODE === true) {
            $m = true;
            if (defined('SNIPPET_INTERACTIVE_MODE') && SNIPPET_INTERACTIVE_MODE == 'true') {
                $m = "interact";
            } else {
                if (defined('SNIPPET_INSTALL_MODE') && SNIPPET_INSTALL_MODE == 'true') {
                    $m = "install";
                }
            }
        }

        return $m;
    }

    /**
     * @deprecated
     *
     * @param $chunkName
     * @return bool|string
     */
    public function putChunk($chunkName)
    { // alias name >.<
        $modx = evolutionCMS();

        return $modx->getChunk($chunkName);
    }

    /**
     * @deprecated
     *
     * @return array|string
     */
    public function getDocGroups()
    {
        $modx = evolutionCMS();

        return $modx->getUserDocGroups();
    }

    /**
     * @deprecated
     *
     * @param string $o
     * @param string $n
     * @return bool|string
     */
    public function changePassword($o, $n)

    {
        $modx = evolutionCMS();

        return $modx->changeWebUserPassword($o, $n);
    }

    /**
     * @deprecated
     *
     * @return array|bool

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

// Better
class Router
{
    private $host;

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

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

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

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $modx = evolutionCMS();
        $userdetails = array();
        if ($modx->isFrontend() && isset ($_SESSION['webValidated'])) {
            // web user
            $userdetails['loggedIn'] = true;
            $userdetails['id'] = $_SESSION['webInternalKey'];
            $userdetails['username'] = $_SESSION['webShortname'];
            $userdetails['usertype'] = 'web'; // added by Raymond

            return $userdetails;
        } else {
            if ($modx->isBackend() && isset ($_SESSION['mgrValidated'])) {
                // manager user
                $userdetails['loggedIn'] = true;
                $userdetails['id'] = $_SESSION['mgrInternalKey'];
                $userdetails['username'] = $_SESSION['mgrShortname'];
                $userdetails['usertype'] = 'manager'; // added by Raymond

                return $userdetails;
            } else {
                return false;
            }
        }
    }

    /**
     * @deprecated
     *
     * @param string $method
     * @param string $prefix
     * @param string $trim
     * @param $REQUEST_METHOD
     * @return array|bool

     */
    public function getFormVars($method = "", $prefix = "", $trim = "", $REQUEST_METHOD)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

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

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

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        //  function to retrieve form results into an associative array
        $modx = evolutionCMS();
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
        $results = array();
        $method = strtoupper($method);
        if ($method == "") {
            $method = $REQUEST_METHOD;
        }
        if ($method == "POST") {
            $method = &$_POST;
        } elseif ($method == "GET") {
            $method = &$_GET;
        } else {
            return false;
        }
        reset($method);
        foreach ($method as $key => $value) {
            if (($prefix != "") && (substr($key, 0, strlen($prefix)) == $prefix)) {
                if ($trim) {
                    $pieces = explode($prefix, $key, 2);
                    $key = $pieces[1];
                    $results[$key] = $value;
                } else {
                    $results[$key] = $value;
                }
            } elseif ($prefix == "") {
                $results[$key] = $value;
            }
        }

        return $results;
    }

    /**
     * Displays a javascript alert message in the web browser
     *
     * @deprecated
     *
     * @param string $msg Message to show
     * @param string $url URL to redirect to
     */
    public function webAlert($msg, $url = "")
    {
        $modx = evolutionCMS();
        $msg = addslashes($modx->getDatabase()->escape($msg));
        if (substr(strtolower($url), 0, 11) == "javascript:") {
            $act = "__WebAlert();";
            $fnc = "function __WebAlert(){" . substr($url, 11) . "};";
        } else {
            $act = ($url ? "window.location.href='" . addslashes($url) . "';" : "");
        }
        $html = "<script>$fnc window.setTimeout(\"alert('$msg');$act\",100);</script>";
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

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

    // $x is potentially undefined here.
    echo $x;
}
        if ($modx->isFrontend()) {
            $modx->regClientScript($html);
        } else {
            echo $html;
        }
    }

    ########################################

    // END New database functions - rad14701
    ########################################

}


1		<?php namespace EvolutionCMS\Legacy;
2
3		use EvolutionCMS\Interfaces\DeprecatedCoreInterface;
4
5		class DeprecatedCore implements DeprecatedCoreInterface
6		{
7		/**
8		* @deprecated
9		*
10		* return @void
11		*/
12		public function dbConnect()
13		{
14		$modx = evolutionCMS();
15		$modx->getDatabase()->connect();
16		$modx->rs = $modx->getDatabase()->conn;
17		}
18
19		/**
20		* @deprecated
21		*
22		* @param $sql
23		* @return bool\|mysqli_result\|resource
		0 ignored issues – show Documentation introduced 2018-06-12 09:22 UTC by Report Bug Copy Issue Report Should the return type not be `boolean\|\mysqli_result`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
24		*/
25		public function dbQuery($sql)
26		{
27		$modx = evolutionCMS();
28
29		return $modx->getDatabase()->query($sql);
30		}
31
32		/**
33		* @deprecated
34		*
35		* @param $rs
36		* @return int
37		*/
38		public function recordCount($rs)
		0 ignored issues – show Comprehensibility introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report Avoid variables with short names like $rs. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history...
39		{
40		$modx = evolutionCMS();
41
42		return $modx->getDatabase()->getRecordCount($rs);
43		}
44
45		/**
46		* @deprecated
47		*
48		* @param $rs
49		* @param string $mode
50		* @return array\|bool\|mixed\|object\|stdClass
51		*/
52		public function fetchRow($rs, $mode = 'assoc')
		0 ignored issues – show Comprehensibility introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report Avoid variables with short names like $rs. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history...
53		{
54		$modx = evolutionCMS();
55
56		return $modx->getDatabase()->getRow($rs, $mode);
57		}
58
59		/**
60		* @deprecated
61		*
62		* @param $rs
63		* @return int
64		*/
65		public function affectedRows($rs)
		0 ignored issues – show Comprehensibility introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report Avoid variables with short names like $rs. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history...
66		{
67		$modx = evolutionCMS();
68
69		return $modx->getDatabase()->getAffectedRows($rs);
70		}
71
72		/**
73		* @deprecated
74		*
75		* @param $rs
76		* @return int\|mixed
77		*/
78		public function insertId($rs)
		0 ignored issues – show Comprehensibility introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report Avoid variables with short names like $rs. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history...
79		{
80		$modx = evolutionCMS();
81
82		return $modx->getDatabase()->getInsertId($rs);
83		}
84
85		/**
86		* @deprecated
87		*
88		* @return void
89		*/
90		public function dbClose()
91		{
92		$modx = evolutionCMS();
93		$modx->getDatabase()->disconnect();
94		}
95
96		/**
97		* @deprecated
98		*
99		* @param array $array
100		* @param string $ulroot
101		* @param string $ulprefix
102		* @param string $type
103		* @param bool $ordered
104		* @param int $tablevel
105		* @return string
106		*/
107		public function makeList($array, $ulroot = 'root', $ulprefix = 'sub_', $type = '', $ordered = false, $tablevel = 0)
108		{
109		// first find out whether the value passed is an array
110		if (!is_array($array)) {
111		return "<ul><li>Bad list</li></ul>";
112		}
113		if (!empty ($type)) {
114		$typestr = " style='list-style-type: $type'";
115		} else {
116		$typestr = "";
117		}
118		$tabs = "";
119		for ($i = 0; $i < $tablevel; $i++) {
120		$tabs .= "\t";
121		}
122		$listhtml = $ordered == true ? $tabs . "<ol class='$ulroot'$typestr>\n" : $tabs . "<ul class='$ulroot'$typestr>\n";
		0 ignored issues – show Coding Style Best Practice introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report It seems like you are loosely comparing two booleans. Considering using the strict comparison `===` instead. When comparing two booleans, it is generally considered safer to use the strict comparison operator. Loading history...
123		foreach ($array as $key => $value) {
124		if (is_array($value)) {
125		$listhtml .= $tabs . "\t<li>" . $key . "\n" . $this->makeList($value, $ulprefix . $ulroot, $ulprefix,
		0 ignored issues – show Deprecated Code introduced 2018-06-12 09:22 UTC by Report Bug Copy Issue Report The method `EvolutionCMS\Legacy\DeprecatedCore::makeList()` has been deprecated. This method has been deprecated. Loading history...
126		$type, $ordered, $tablevel + 2) . $tabs . "\t</li>\n";
127		} else {
128		$listhtml .= $tabs . "\t<li>" . $value . "</li>\n";
129		}
130		}
131		$listhtml .= $ordered == true ? $tabs . "</ol>\n" : $tabs . "</ul>\n";
		0 ignored issues – show Coding Style Best Practice introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report It seems like you are loosely comparing two booleans. Considering using the strict comparison `===` instead. When comparing two booleans, it is generally considered safer to use the strict comparison operator. Loading history...
132
133		return $listhtml;
134		}
135
136		/**
137		* @deprecated
138		*
139		* @return array
140		*/
141		public function getUserData()
		0 ignored issues – show Coding Style introduced 2018-02-25 17:05 UTC by Report Bug Copy Issue Report `getUserData` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
142		{
143		$client = array();
144		$client['ip'] = $_SERVER['REMOTE_ADDR'];
145		$client['ua'] = $_SERVER['HTTP_USER_AGENT'];
146
147		return $client;
148		}
149
150		/**
151		* Returns true, install or interact when inside manager
152		*
153		* @deprecated
154		*
155		* @return bool\|string
156		*/
157		public function insideManager()
158		{
159		$m = false;
		0 ignored issues – show Comprehensibility introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report Avoid variables with short names like $m. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history...
160		if (defined('IN_MANAGER_MODE') && IN_MANAGER_MODE === true) {
161		$m = true;
162		if (defined('SNIPPET_INTERACTIVE_MODE') && SNIPPET_INTERACTIVE_MODE == 'true') {
163		$m = "interact";
164		} else {
165		if (defined('SNIPPET_INSTALL_MODE') && SNIPPET_INSTALL_MODE == 'true') {
166		$m = "install";
167		}
168		}
169		}
170
171		return $m;
172		}
173
174		/**
175		* @deprecated
176		*
177		* @param $chunkName
178		* @return bool\|string
179		*/
180		public function putChunk($chunkName)
181		{ // alias name >.<
182		$modx = evolutionCMS();
183
184		return $modx->getChunk($chunkName);
185		}
186
187		/**
188		* @deprecated
189		*
190		* @return array\|string
191		*/
192		public function getDocGroups()
193		{
194		$modx = evolutionCMS();
195
196		return $modx->getUserDocGroups();
197		}
198
199		/**
200		* @deprecated
201		*
202		* @param string $o
203		* @param string $n
204		* @return bool\|string
205		*/
206		public function changePassword($o, $n)
		0 ignored issues – show Comprehensibility introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report Avoid variables with short names like $o. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history... Comprehensibility introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report Avoid variables with short names like $n. Configured minimum length is 3. Short variable names may make your code harder to understand. Variable names should be self-descriptive. This check looks for variable names who are shorter than a configured minimum. Loading history...
207		{
208		$modx = evolutionCMS();
209
210		return $modx->changeWebUserPassword($o, $n);
211		}
212
213		/**
214		* @deprecated
215		*
216		* @return array\|bool
		0 ignored issues – show Documentation introduced 2018-02-25 17:05 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `array\|false`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
217		*/
218		public function userLoggedIn()
		0 ignored issues – show Coding Style introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report `userLoggedIn` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
219		{
220		$modx = evolutionCMS();
221		$userdetails = array();
222		if ($modx->isFrontend() && isset ($_SESSION['webValidated'])) {
223		// web user
224		$userdetails['loggedIn'] = true;
225		$userdetails['id'] = $_SESSION['webInternalKey'];
226		$userdetails['username'] = $_SESSION['webShortname'];
227		$userdetails['usertype'] = 'web'; // added by Raymond
228
229		return $userdetails;
230		} else {
231		if ($modx->isBackend() && isset ($_SESSION['mgrValidated'])) {
232		// manager user
233		$userdetails['loggedIn'] = true;
234		$userdetails['id'] = $_SESSION['mgrInternalKey'];
235		$userdetails['username'] = $_SESSION['mgrShortname'];
236		$userdetails['usertype'] = 'manager'; // added by Raymond
237
238		return $userdetails;
239		} else {
240		return false;
241		}
242		}
243		}
244
245		/**
246		* @deprecated
247		*
248		* @param string $method
249		* @param string $prefix
250		* @param string $trim
251		* @param $REQUEST_METHOD
252		* @return array\|bool
		0 ignored issues – show Documentation introduced 2018-02-25 17:05 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `false\|array`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
253		*/
254		public function getFormVars($method = "", $prefix = "", $trim = "", $REQUEST_METHOD)
		0 ignored issues – show Coding Style introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report `getFormVars` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report `getFormVars` uses the super-global variable `$_GET` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style Naming introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report The parameter $REQUEST_METHOD is not named in camelCase. This check marks parameter names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history... Coding Style introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report Parameters which have default values should be placed at the end. If you place a parameter with a default value before a parameter with a default value, the default value of the first parameter will never be used as it will always need to be passed anyway: // $a must always be passed; it's default value is never used. function someFunction($a = 5, $b) { } Loading history...
255		{
256		// function to retrieve form results into an associative array
257		$modx = evolutionCMS();
		0 ignored issues – show Unused Code introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report `$modx` 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...
258		$results = array();
259		$method = strtoupper($method);
260		if ($method == "") {
261		$method = $REQUEST_METHOD;
262		}
263		if ($method == "POST") {
264		$method = &$_POST;
265		} elseif ($method == "GET") {
266		$method = &$_GET;
267		} else {
268		return false;
269		}
270		reset($method);
271		foreach ($method as $key => $value) {
272		if (($prefix != "") && (substr($key, 0, strlen($prefix)) == $prefix)) {
273		if ($trim) {
274		$pieces = explode($prefix, $key, 2);
275		$key = $pieces[1];
276		$results[$key] = $value;
277		} else {
278		$results[$key] = $value;
279		}
280		} elseif ($prefix == "") {
281		$results[$key] = $value;
282		}
283		}
284
285		return $results;
286		}
287
288		/**
289		* Displays a javascript alert message in the web browser
290		*
291		* @deprecated
292		*
293		* @param string $msg Message to show
294		* @param string $url URL to redirect to
295		*/
296		public function webAlert($msg, $url = "")
297		{
298		$modx = evolutionCMS();
299		$msg = addslashes($modx->getDatabase()->escape($msg));
300	View Code Duplication	if (substr(strtolower($url), 0, 11) == "javascript:") {
301		$act = "__WebAlert();";
302		$fnc = "function __WebAlert(){" . substr($url, 11) . "};";
303		} else {
304		$act = ($url ? "window.location.href='" . addslashes($url) . "';" : "");
305		}
306		$html = "<script>$fnc window.setTimeout(\"alert('$msg');$act\",100);</script>";
		0 ignored issues – show Bug introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report The variable `$fnc` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
307		if ($modx->isFrontend()) {
308		$modx->regClientScript($html);
309		} else {
310		echo $html;
311		}
312		}
313
314		########################################
		0 ignored issues – show Coding Style introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report Perl-style comments are not allowed. Use "// Comment." or "/* comment */" instead. Loading history...
315		// END New database functions - rad14701
316		########################################
		0 ignored issues – show Coding Style introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report Perl-style comments are not allowed. Use "// Comment." or "/* comment */" instead. Loading history...
317		}
318

evolution-cms / evolution