SQLDataSet - Code Metrics - Inspection of "Fix missing ;" - BurningFlipside/CommonCode - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 3e714a...9b3c62 )

by Patrick

created 2016-07-05 20:19 UTC

SQLDataSet B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	258
Duplicated Lines	34.88 %

Coupling/Cohesion

Components	1
Dependencies	2

Importance

Changes	5
Bugs	3	Features	2

Metric	Value
dl	90
loc	258
rs	8.5454
c	5
b	3
f	2
wmc	49
lcom	1
cbo	2

14 Methods

Rating	Name	Duplication	Size	Complexity
A	_get_row_count_for_query()	0	15	3
A	_tableExistsNoPrefix()	12	12	3
A	_tableExists()	12	12	3
A	_viewExists()	12	12	3
A	tableExists()	0	16	4
A	getTable()	0	16	4
A	__construct()	11	11	2
A	getOrderByClause()	0	15	4
A	getLimitClause()	0	14	3
C	read()	0	33	8
A	update()	21	21	4
B	create()	22	22	4
A	delete()	0	9	2
A	raw_query()	0	10	2

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

<?php
namespace Data;

class SQLDataSet extends DataSet
{
    protected $pdo;

    public function __construct($params)

    {
        if(isset($params['user']))
        {
            $this->pdo = new \PDO($params['dsn'], $params['user'], $params['pass']);
        }
        else
        {
            $this->pdo = new \PDO($params['dsn']);
        }
    }

    /**
     * Get the number of rows affected by the query
     *
     * @param string $sql The SQL string
     *
     * @return integer The number of rows affected by the query
     */
    private function _get_row_count_for_query($sql)
    {
        $stmt = $this->pdo->query($sql);
        if($stmt === false)
        {
            return 0;
        }
        $count = $stmt->rowCount();
        if($count === 0)
        {
            $array = $stmt->fetchAll();
            $count = count($array);
        }
        return $count;
    }

    function _tableExistsNoPrefix($name)

    {
        if($this->_get_row_count_for_query('SHOW TABLES LIKE '.$this->pdo->quote($name)) > 0)
        {
            return true;
        }
        else if($this->_get_row_count_for_query('SELECT * FROM sqlite_master WHERE name LIKE '.$this->pdo->quote($name)) > 0)
        {
            return true;
        }
        return false;
    }

    function _tableExists($name)

    {
        if($this->_get_row_count_for_query('SHOW TABLES LIKE '.$this->pdo->quote('tbl'.$name)) > 0)
        {
            return true;
        }
        else if($this->_get_row_count_for_query('SELECT * FROM sqlite_master WHERE name LIKE '.$this->pdo->quote('tbl'.$name)) > 0)
        {
            return true;
        }
        return false;
    }

    function _viewExists($name)

    {
        if($this->_get_row_count_for_query('SHOW TABLES LIKE '.$this->pdo->quote('v'.$name)) > 0)
        {
            return true;
        }
        else if($this->_get_row_count_for_query('SELECT * FROM sqlite_master WHERE name LIKE '.$this->pdo->quote('v'.$name)) > 0)
        {
            return true;
        }
        return false;
    }

    function tableExists($name)

    {
        if($this->_tableExists($name))
        {
            return true;
        }
        if($this->_tableExistsNoPrefix($name))
        {
            return true;
        }
        if($this->_viewExists($name))
        {
            return true;
        }
        return false;
    }

    function getTable($name)

    {
        if($this->_tableExists($name))
        {
            return new SQLDataTable($this, 'tbl'.$name);
        }
        if($this->_viewExists($name))
        {
            return new SQLDataTable($this, 'v'.$name);
        }
        if($this->_tableExistsNoPrefix($name))
        {
            return new SQLDataTable($this, $name);
        }
        throw new \Exception('No such table '.$name);
    }

    /**
     * @param array $sort The array to sort by or false to not sort
     */
    private function getOrderByClause($sort)
    {
        if(empty($sort))
        {
            return false;
        }
        $sql = ' ORDER BY ';
        $tmp = array();
        foreach($sort as $sort_col=>$dir)
        {
            array_push($tmp, $sort_col.' '.($dir === 1 ? 'ASC' : 'DESC'));
        }
        $sql .= implode($tmp, ',');
        return $sql;
    }

    private function getLimitClause($count, $skip)
    {
        if($count === false)
        {
            return false;
        }
        $count = intval($count);
        if($skip !== false)
        {
            $skip = intval($count);
            return " LIMIT $skip, $count";
        }
        return ' LIMIT '.$count;
    }

    /**
     * Read data from the specified SQL table
     *
     * @param string $tablename The name of the table to read from
     * @param string $where The where caluse of the SQL statement
     * @param string $select The colums to read
     * @param string $count The number of rows to read
     * @param string $skip The number of rows to skip over
     * @param array $sort The array to sort by or false to not sort
     *
     * @return array An array of all the returned records
     */
    public function read($tablename, $where = false, $select = '*', $count = false, $skip = false, $sort = false)
    {
        if($select === false)
        {
            $select = '*';
        }
        $sql = "SELECT $select FROM $tablename";
        if($where !== false)
        {
            $sql .= ' WHERE '.$where;
        }
        $tmp = $this->getLimitClause($count, $skip);
        if($tmp !== false)
        {
            $sql .= $tmp;
        }
        $tmp = $this->getOrderByClause($sort);

        if($tmp !== false)
        {
            $sql .= $tmp;
        }
        $stmt = $this->pdo->query($sql, \PDO::FETCH_ASSOC);
        if($stmt === false)
        {
            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());
}
        }
        $ret = $stmt->fetchAll();
        if($ret === false || empty($ret))
        {
            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());
}
        }
        return $ret;
    }

    function update($tablename, $where, $data)

    {
        $set = array();
        if(is_object($data))
        {
            $data = (array)$data;
        }
        $cols = array_keys($data);
        $count = count($cols);
        for($i = 0; $i < $count; $i++)
        {
            array_push($set, $cols[$i].'='.$this->pdo->quote($data[$cols[$i]]));
        }
        $set = implode(',', $set);
        $sql = "UPDATE $tablename SET $set WHERE $where";
        if($this->pdo->exec($sql) === false)
        {
            return false;
        }
        return true;
    }

    function create($tablename, $data)

    {
        $set = array();
        if(is_object($data))
        {
            $data = (array)$data;
        }
        $cols = array_keys($data);
        $count = count($cols);
        for($i = 0; $i < $count; $i++)
        {
            array_push($set, $this->pdo->quote($data[$cols[$i]]));
        }
        $cols = implode(',', $cols);
        $set = implode(',', $set);
        $sql = "INSERT INTO $tablename ($cols) VALUES ($set);";
        if($this->pdo->exec($sql) === false)
        {
            return false;
        }
        return true;
    }

    function delete($tablename, $where)

    {
        $sql = "DELETE FROM $tablename WHERE $where";
        if($this->pdo->exec($sql) === false)
        {
            return false;
        }
        return true;
    }

    function raw_query($sql)

    {
        $stmt = $this->pdo->query($sql, \PDO::FETCH_ASSOC);
        if($stmt === false)
        {
            return false;
        }
        $ret = $stmt->fetchAll();
        return $ret;
    }
}
/* vim: set tabstop=4 shiftwidth=4 expandtab: */


1		<?php
2		namespace Data;
3
4		class SQLDataSet extends DataSet
5		{
6		protected $pdo;
7
8	View Code Duplication	public function __construct($params)
		0 ignored issues – show Duplication introduced 2016-05-13 19:31 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...
9		{
10		if(isset($params['user']))
11		{
12		$this->pdo = new \PDO($params['dsn'], $params['user'], $params['pass']);
13		}
14		else
15		{
16		$this->pdo = new \PDO($params['dsn']);
17		}
18		}
19
20		/**
21		* Get the number of rows affected by the query
22		*
23		* @param string $sql The SQL string
24		*
25		* @return integer The number of rows affected by the query
26		*/
27		private function _get_row_count_for_query($sql)
28		{
29		$stmt = $this->pdo->query($sql);
30		if($stmt === false)
31		{
32		return 0;
33		}
34		$count = $stmt->rowCount();
35		if($count === 0)
36		{
37		$array = $stmt->fetchAll();
38		$count = count($array);
39		}
40		return $count;
41		}
42
43	View Code Duplication	function _tableExistsNoPrefix($name)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Duplication introduced 2016-05-13 18:20 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...
44		{
45		if($this->_get_row_count_for_query('SHOW TABLES LIKE '.$this->pdo->quote($name)) > 0)
46		{
47		return true;
48		}
49		else if($this->_get_row_count_for_query('SELECT * FROM sqlite_master WHERE name LIKE '.$this->pdo->quote($name)) > 0)
50		{
51		return true;
52		}
53		return false;
54		}
55
56	View Code Duplication	function _tableExists($name)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Duplication introduced 2016-05-13 18:20 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...
57		{
58		if($this->_get_row_count_for_query('SHOW TABLES LIKE '.$this->pdo->quote('tbl'.$name)) > 0)
59		{
60		return true;
61		}
62		else if($this->_get_row_count_for_query('SELECT * FROM sqlite_master WHERE name LIKE '.$this->pdo->quote('tbl'.$name)) > 0)
63		{
64		return true;
65		}
66		return false;
67		}
68
69	View Code Duplication	function _viewExists($name)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Duplication introduced 2016-05-13 18:20 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...
70		{
71		if($this->_get_row_count_for_query('SHOW TABLES LIKE '.$this->pdo->quote('v'.$name)) > 0)
72		{
73		return true;
74		}
75		else if($this->_get_row_count_for_query('SELECT * FROM sqlite_master WHERE name LIKE '.$this->pdo->quote('v'.$name)) > 0)
76		{
77		return true;
78		}
79		return false;
80		}
81
82		function tableExists($name)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
83		{
84		if($this->_tableExists($name))
85		{
86		return true;
87		}
88		if($this->_tableExistsNoPrefix($name))
89		{
90		return true;
91		}
92		if($this->_viewExists($name))
93		{
94		return true;
95		}
96		return false;
97		}
98
99		function getTable($name)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
100		{
101		if($this->_tableExists($name))
102		{
103		return new SQLDataTable($this, 'tbl'.$name);
104		}
105		if($this->_viewExists($name))
106		{
107		return new SQLDataTable($this, 'v'.$name);
108		}
109		if($this->_tableExistsNoPrefix($name))
110		{
111		return new SQLDataTable($this, $name);
112		}
113		throw new \Exception('No such table '.$name);
114		}
115
116		/**
117		* @param array $sort The array to sort by or false to not sort
118		*/
119		private function getOrderByClause($sort)
120		{
121		if(empty($sort))
122		{
123		return false;
124		}
125		$sql = ' ORDER BY ';
126		$tmp = array();
127		foreach($sort as $sort_col=>$dir)
128		{
129		array_push($tmp, $sort_col.' '.($dir === 1 ? 'ASC' : 'DESC'));
130		}
131		$sql .= implode($tmp, ',');
132		return $sql;
133		}
134
135		private function getLimitClause($count, $skip)
136		{
137		if($count === false)
138		{
139		return false;
140		}
141		$count = intval($count);
142		if($skip !== false)
143		{
144		$skip = intval($count);
145		return " LIMIT $skip, $count";
146		}
147		return ' LIMIT '.$count;
148		}
149
150		/**
151		* Read data from the specified SQL table
152		*
153		* @param string $tablename The name of the table to read from
154		* @param string $where The where caluse of the SQL statement
155		* @param string $select The colums to read
156		* @param string $count The number of rows to read
157		* @param string $skip The number of rows to skip over
158		* @param array $sort The array to sort by or false to not sort
159		*
160		* @return array An array of all the returned records
161		*/
162		public function read($tablename, $where = false, $select = '*', $count = false, $skip = false, $sort = false)
163		{
164		if($select === false)
165		{
166		$select = '*';
167		}
168		$sql = "SELECT $select FROM $tablename";
169		if($where !== false)
170		{
171		$sql .= ' WHERE '.$where;
172		}
173		$tmp = $this->getLimitClause($count, $skip);
174		if($tmp !== false)
175		{
176		$sql .= $tmp;
177		}
178		$tmp = $this->getOrderByClause($sort);
		0 ignored issues – show Bug introduced 2016-07-05 20:22 UTC by Report Bug Copy Issue Report It seems like `$sort` defined by parameter `$sort` on line 162 can also be of type `false`; however, `Data\SQLDataSet::getOrderByClause()` does only seem to accept `array`, maybe add an additional type check? This check looks at variables that have been passed in as parameters and are passed out again to other methods. If the outgoing method call has stricter type requirements than the method itself, an issue is raised. An additional type check may prevent trouble. Loading history...
179		if($tmp !== false)
180		{
181		$sql .= $tmp;
182		}
183		$stmt = $this->pdo->query($sql, \PDO::FETCH_ASSOC);
184		if($stmt === false)
185		{
186		return false;
		0 ignored issues – show Bug Best Practice introduced 2016-07-05 20:22 UTC by Report Bug Copy Issue Report The return type of `return false;` (`false`) is incompatible with the return type documented by `Data\SQLDataSet::read` of type `array`. 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...
187		}
188		$ret = $stmt->fetchAll();
189		if($ret === false \|\| empty($ret))
190		{
191		return false;
		0 ignored issues – show Bug Best Practice introduced 2016-07-05 20:22 UTC by Report Bug Copy Issue Report The return type of `return false;` (`false`) is incompatible with the return type documented by `Data\SQLDataSet::read` of type `array`. 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...
192		}
193		return $ret;
194		}
195
196	View Code Duplication	function update($tablename, $where, $data)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Duplication introduced 2016-05-13 18:20 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...
197		{
198		$set = array();
199		if(is_object($data))
200		{
201		$data = (array)$data;
202		}
203		$cols = array_keys($data);
204		$count = count($cols);
205		for($i = 0; $i < $count; $i++)
206		{
207		array_push($set, $cols[$i].'='.$this->pdo->quote($data[$cols[$i]]));
208		}
209		$set = implode(',', $set);
210		$sql = "UPDATE $tablename SET $set WHERE $where";
211		if($this->pdo->exec($sql) === false)
212		{
213		return false;
214		}
215		return true;
216		}
217
218	View Code Duplication	function create($tablename, $data)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Duplication introduced 2016-05-13 18:20 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...
219		{
220		$set = array();
221		if(is_object($data))
222		{
223		$data = (array)$data;
224		}
225		$cols = array_keys($data);
226		$count = count($cols);
227		for($i = 0; $i < $count; $i++)
228		{
229		array_push($set, $this->pdo->quote($data[$cols[$i]]));
230		}
231		$cols = implode(',', $cols);
232		$set = implode(',', $set);
233		$sql = "INSERT INTO $tablename ($cols) VALUES ($set);";
234		if($this->pdo->exec($sql) === false)
235		{
236		return false;
237		}
238		return true;
239		}
240
241		function delete($tablename, $where)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
242		{
243		$sql = "DELETE FROM $tablename WHERE $where";
244		if($this->pdo->exec($sql) === false)
245		{
246		return false;
247		}
248		return true;
249		}
250
251		function raw_query($sql)
		0 ignored issues – show Best Practice introduced 2016-05-13 18:20 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
252		{
253		$stmt = $this->pdo->query($sql, \PDO::FETCH_ASSOC);
254		if($stmt === false)
255		{
256		return false;
257		}
258		$ret = $stmt->fetchAll();
259		return $ret;
260		}
261		}
262		/* vim: set tabstop=4 shiftwidth=4 expandtab: */
263

BurningFlipside / CommonCode