Issues in NotebookMapper.php - Failed Issues - Inspection of "Backend update" - brantje/nextnote - Measure and Improve Code Quality continuously with Scrutinizer

Failed Conditions

Pull Request — master (#72)

by Sander

created 2018-04-10 20:30 UTC

lib/Db/NotebookMapper.php (3 issues)

Labels

Severity

Major 3

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
/**
 * Nextcloud - NextNote
 *
 * @copyright Copyright (c) 2015, Ben Curtis <[email protected]>
 * @copyright Copyright (c) 2017, Sander Brand ([email protected])
 * @license GNU AGPL version 3 or any later version
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License as
 * published by the Free Software Foundation, either version 3 of the
 * License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *
 */

namespace OCA\NextNote\Db;

use \OCA\NextNote\Utility\Utils;
use OCP\AppFramework\Db\Entity;
use OCP\IDBConnection;
use OCP\AppFramework\Db\Mapper;

class NotebookMapper extends Mapper {
	private $utils;

	public function __construct(IDBConnection $db, Utils $utils) {
		parent::__construct($db, 'nextnote_groups');
		$this->utils = $utils;
	}

	/**
	 * Get Notebook(s)
	 * @param int $notebook_id
	 * @param null|int $user_id
	 * @param bool|int $deleted
	 * @return Notebook[]|Notebook
	 */
	public function find($notebook_id, $user_id = null, $deleted = false) {
		$params = [];
		$where = [];
		if($notebook_id){
			$where[] = 'g.id= ?';
			$params[] = $notebook_id;
		}

		if ($user_id) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
			$params[] = $user_id;
			$where[] = 'g.uid = ?';
		}

		if ($deleted !== false) {
			$params[] = $deleted;
			$where[] = 'g.deleted = ?';
		}
		$where = implode(' AND ', $where);
		if($where){
			$where = 'WHERE '. $where;
		}
		$sql = "SELECT g.*, g.guid as guid, COUNT(n.id) as note_count FROM *PREFIX*nextnote_groups g LEFT JOIN *PREFIX*nextnote_notes n ON g.name=n.grouping $where  GROUP BY g.id";
		$results = [];
		foreach ($this->execute($sql, $params)->fetchAll() as $item) {
			$results[] = $this->makeEntityFromDBResult($item);
		}
//		var_dump($results);
		if(count($results) === 1){
			return reset($results);
		}

		return $results;
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());
}
	}

	/**
	 * @param $group_name
	 * @param null $user_id
	 * @param bool $deleted
	 * @return Notebook[]|Notebook
	 */
	public function findByName($group_name, $user_id = null, $deleted = false) {
		$params = [];
		$where = [];
		if($group_name){
			$where[] = 'g.name = ?';
			$params[] = $group_name;
		}

		if ($user_id) {
			$params[] = $user_id;
			$where[] = 'g.uid = ?';
		}

		if ($deleted !== false) {
			$params[] = $deleted;
			$where[] = 'g.deleted = ?';
		}
		$where = implode(' AND ', $where);
		if($where){
			$where = 'WHERE '. $where;
		}
		$sql = "SELECT g.*, COUNT(n.id) as note_count FROM *PREFIX*nextnote_groups g LEFT JOIN *PREFIX*nextnote_notes n ON g.name=n.grouping $where  GROUP BY g.id";
		$results = [];
		foreach ($this->execute($sql, $params)->fetchAll() as $item) {
			$results[] = $this->makeEntityFromDBResult($item);
		}

		if(count($results) === 1){
			return reset($results);
		}

		return $results;
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());
}
	}

   	/**
	 * Creates a group
	 *
	 * @param Notebook|Entity $group
	 * @return Note|Entity
	 * @internal param $userId
	 */
	public function insert(Entity $group) {
		$group->setNoteCount(null);
		return parent::insert($group);
	}

	/**
	 * Update group
	 *
	 * @param Notebook|Entity $group
	 * @return Notebook|Entity
	 */
	public function update(Entity $group) {
		$group->setNoteCount(null);
		return parent::update($group);
	}

	/**
	 * Delete group
	 *
	 * @param Notebook|Entity $group
	 * @return Notebook|Entity
	 */
	public function delete(Entity $group) {
		return parent::delete($group);
	}

	/**
	 * @param $arr
	 * @return Notebook
	 */
	public function makeEntityFromDBResult($arr) {
		$group = new Notebook();
		$group->setId($arr['id']);
		$group->setName($arr['name']);
		$group->setGuid($arr['guid']);
		$group->setParentId($arr['parent_id']);
		$group->setColor($arr['color']);
		$group->setNoteCount($arr['note_count']);
		$group->setDeleted($arr['deleted']);

		return $group;
	}


}


1		<?php
2		/**
3		* Nextcloud - NextNote
4		*
5		* @copyright Copyright (c) 2015, Ben Curtis <[email protected]>
6		* @copyright Copyright (c) 2017, Sander Brand ([email protected])
7		* @license GNU AGPL version 3 or any later version
8		*
9		* This program is free software: you can redistribute it and/or modify
10		* it under the terms of the GNU Affero General Public License as
11		* published by the Free Software Foundation, either version 3 of the
12		* License, or (at your option) any later version.
13		*
14		* This program is distributed in the hope that it will be useful,
15		* but WITHOUT ANY WARRANTY; without even the implied warranty of
16		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17		* GNU Affero General Public License for more details.
18		*
19		* You should have received a copy of the GNU Affero General Public License
20		* along with this program. If not, see <http://www.gnu.org/licenses/>.
21		*
22		*/
23
24		namespace OCA\NextNote\Db;
25
26		use \OCA\NextNote\Utility\Utils;
27		use OCP\AppFramework\Db\Entity;
28		use OCP\IDBConnection;
29		use OCP\AppFramework\Db\Mapper;
30
31		class NotebookMapper extends Mapper {
32		private $utils;
33
34		public function __construct(IDBConnection $db, Utils $utils) {
35		parent::__construct($db, 'nextnote_groups');
36		$this->utils = $utils;
37		}
38
39		/**
40		* Get Notebook(s)
41		* @param int $notebook_id
42		* @param null\|int $user_id
43		* @param bool\|int $deleted
44		* @return Notebook[]\|Notebook
45		*/
46	View Code Duplication	public function find($notebook_id, $user_id = null, $deleted = false) {
47		$params = [];
48		$where = [];
49		if($notebook_id){
50		$where[] = 'g.id= ?';
51		$params[] = $notebook_id;
52		}
53
54		if ($user_id) {
		0 ignored issues – show Bug Best Practice introduced 2018-04-10 20:32 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$user_id` of type `null\|integer` is loosely compared to `true`; this is ambiguous if the integer can be zero. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
55		$params[] = $user_id;
56		$where[] = 'g.uid = ?';
57		}
58
59		if ($deleted !== false) {
60		$params[] = $deleted;
61		$where[] = 'g.deleted = ?';
62		}
63		$where = implode(' AND ', $where);
64		if($where){
65		$where = 'WHERE '. $where;
66		}
67		$sql = "SELECT g., g.guid as guid, COUNT(n.id) as note_count FROM PREFIXnextnote_groups g LEFT JOIN PREFIX*nextnote_notes n ON g.name=n.grouping $where GROUP BY g.id";
68		$results = [];
69		foreach ($this->execute($sql, $params)->fetchAll() as $item) {
70		$results[] = $this->makeEntityFromDBResult($item);
71		}
72		// var_dump($results);
73		if(count($results) === 1){
74		return reset($results);
75		}
76
77		return $results;
		0 ignored issues – show Bug Best Practice introduced 2018-04-10 20:32 UTC by Report Bug Copy Issue Report Show Similar Issues like this The return type of `return $results;` (`array`) is incompatible with the return type documented by `OCA\NextNote\Db\NotebookMapper::find` of type `OCA\NextNote\Db\Notebook...CA\NextNote\Db\Notebook`. 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...
78		}
79
80		/**
81		* @param $group_name
82		* @param null $user_id
83		* @param bool $deleted
84		* @return Notebook[]\|Notebook
85		*/
86	View Code Duplication	public function findByName($group_name, $user_id = null, $deleted = false) {
87		$params = [];
88		$where = [];
89		if($group_name){
90		$where[] = 'g.name = ?';
91		$params[] = $group_name;
92		}
93
94		if ($user_id) {
95		$params[] = $user_id;
96		$where[] = 'g.uid = ?';
97		}
98
99		if ($deleted !== false) {
100		$params[] = $deleted;
101		$where[] = 'g.deleted = ?';
102		}
103		$where = implode(' AND ', $where);
104		if($where){
105		$where = 'WHERE '. $where;
106		}
107		$sql = "SELECT g., COUNT(n.id) as note_count FROM PREFIXnextnote_groups g LEFT JOIN PREFIX*nextnote_notes n ON g.name=n.grouping $where GROUP BY g.id";
108		$results = [];
109		foreach ($this->execute($sql, $params)->fetchAll() as $item) {
110		$results[] = $this->makeEntityFromDBResult($item);
111		}
112
113		if(count($results) === 1){
114		return reset($results);
115		}
116
117		return $results;
		0 ignored issues – show Bug Best Practice introduced 2018-04-10 20:32 UTC by Report Bug Copy Issue Report Show Similar Issues like this The return type of `return $results;` (`array`) is incompatible with the return type documented by `OCA\NextNote\Db\NotebookMapper::findByName` of type `OCA\NextNote\Db\Notebook...CA\NextNote\Db\Notebook`. 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...
118		}
119
120		/**
121		* Creates a group
122		*
123		* @param Notebook\|Entity $group
124		* @return Note\|Entity
125		* @internal param $userId
126		*/
127		public function insert(Entity $group) {
128		$group->setNoteCount(null);
129		return parent::insert($group);
130		}
131
132		/**
133		* Update group
134		*
135		* @param Notebook\|Entity $group
136		* @return Notebook\|Entity
137		*/
138		public function update(Entity $group) {
139		$group->setNoteCount(null);
140		return parent::update($group);
141		}
142
143		/**
144		* Delete group
145		*
146		* @param Notebook\|Entity $group
147		* @return Notebook\|Entity
148		*/
149		public function delete(Entity $group) {
150		return parent::delete($group);
151		}
152
153		/**
154		* @param $arr
155		* @return Notebook
156		*/
157		public function makeEntityFromDBResult($arr) {
158		$group = new Notebook();
159		$group->setId($arr['id']);
160		$group->setName($arr['name']);
161		$group->setGuid($arr['guid']);
162		$group->setParentId($arr['parent_id']);
163		$group->setColor($arr['color']);
164		$group->setNoteCount($arr['note_count']);
165		$group->setDeleted($arr['deleted']);
166
167		return $group;
168		}
169
170
171		}
172

brantje / nextnote

Pull Request — master (#72)

lib/Db/NotebookMapper.php (3 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like