StackService - Code Metrics - Inspection of "Merge pull request #535 from nextcloud/feature/83/..." - nextcloud/deck - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( a3a70f...f040df )

by Julius

created 2018-08-15 19:21 UTC

StackService A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	276
Duplicated Lines	22.46 %

Coupling/Cohesion

Components	1
Dependencies	11

Importance

Changes

Metric	Value
wmc	40
lcom	1
cbo	11
dl	62
loc	276
rs	9.2
c	0
b	0
f	0

11 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	21	21	1
A	enrichStackWithCards()	0	13	3
A	enrichStacksWithCards()	0	5	2
A	find()	0	15	3
A	findAll()	0	10	2
A	fetchDeleted()	0	6	1
A	findAllArchived()	0	20	5
B	create()	25	25	6
A	delete()	16	16	2
B	update()	0	29	7
B	reorder()	0	33	8

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

<?php
/**
 * @copyright Copyright (c) 2016 Julius Härtl <[email protected]>
 *
 * @author Julius Härtl <[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\Deck\Service;

use OCA\Deck\Db\Acl;
use OCA\Deck\Db\CardMapper;
use OCA\Deck\Db\BoardMapper;
use OCA\Deck\Db\LabelMapper;
use OCA\Deck\Db\AssignedUsersMapper;
use OCA\Deck\Db\Stack;
use OCA\Deck\Db\StackMapper;
use OCA\Deck\StatusException;
use OCA\Deck\BadRequestException;


class StackService {

	private $stackMapper;
	private $cardMapper;
	private $boardMapper;
	private $labelMapper;
	private $permissionService;
	private $boardService;
	private $cardService;
	private $assignedUsersMapper;
	private $attachmentService;

	public function __construct(

		StackMapper $stackMapper,
		BoardMapper $boardMapper,
		CardMapper $cardMapper,
		LabelMapper $labelMapper,
		PermissionService $permissionService,
		BoardService $boardService,
		CardService $cardService,
		AssignedUsersMapper $assignedUsersMapper,
		AttachmentService $attachmentService
	) {
		$this->stackMapper = $stackMapper;
		$this->boardMapper = $boardMapper;
		$this->cardMapper = $cardMapper;
		$this->labelMapper = $labelMapper;
		$this->permissionService = $permissionService;
		$this->boardService = $boardService;
		$this->cardService = $cardService;
		$this->assignedUsersMapper = $assignedUsersMapper;
		$this->attachmentService = $attachmentService;
	}

	private function enrichStackWithCards($stack) {
		$cards = $this->cardMapper->findAll($stack->id);

		if(is_null($cards)) {
			return;
		}

		foreach ($cards as $card) {
			$this->cardService->enrich($card);
		}

		$stack->setCards($cards);
	}

	private function enrichStacksWithCards($stacks) {
		foreach ($stacks as $stack) {
			$this->enrichStackWithCards($stack);
		}
	}

	/**
	 * @param $stackId
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws \OCP\AppFramework\Db\DoesNotExistException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function find($stackId) {
		if (is_numeric($stackId) === false) {
			throw new BadRequestException('stack id must be a number');
		}

		$stack = $this->stackMapper->find($stackId);
		$cards = $this->cardMapper->findAll($stackId);
		foreach ($cards as $cardIndex => $card) {
			$assignedUsers = $this->assignedUsersMapper->find($card->getId());
			$card->setAssignedUsers($assignedUsers);
			$card->setAttachmentCount($this->attachmentService->count($card->getId()));
		}
		$stack->setCards($cards);
		return $stack;
	}

	/**
	 * @param $boardId
	 * @return array
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws BadRequestException
	 */
	public function findAll($boardId) {
		if (is_numeric($boardId) === false) {
			throw new BadRequestException('boardId must be a number');
		}

		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
		$stacks = $this->stackMapper->findAll($boardId);
		$this->enrichStacksWithCards($stacks);
		return $stacks;
	}

	public function fetchDeleted($boardId) {
		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
		$stacks = $this->stackMapper->findDeleted($boardId);
		$this->enrichStacksWithCards($stacks);
		return $stacks;
	}

	/**
	 * @param $boardId
	 * @return array
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws BadRequestException
	 */
	public function findAllArchived($boardId) {

		if (is_numeric($boardId) === false) {
			throw new BadRequestException('board id must be a number');
		}

		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
		$stacks = $this->stackMapper->findAll($boardId);
		$labels = $this->labelMapper->getAssignedLabelsForBoard($boardId);
		foreach ($stacks as $stackIndex => $stack) {
			$cards = $this->cardMapper->findAllArchived($stack->id);
			foreach ($cards as $cardIndex => $card) {
				if (array_key_exists($card->id, $labels)) {
					$cards[$cardIndex]->setLabels($labels[$card->id]);
				}
			}
			$stacks[$stackIndex]->setCards($cards);
		}
		return $stacks;
	}

	/**
	 * @param $title
	 * @param $boardId
	 * @param integer $order
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws StatusException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\DoesNotExistException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function create($title, $boardId, $order) {


		if ($title === false || $title === null) {
			throw new BadRequestException('title must be provided');
		}

		if (is_numeric($order) === false) {
			throw new BadRequestException('order must be a number');
		}

		if (is_numeric($boardId) === false) {
			throw new BadRequestException('board id must be a number');
		}

		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE);
		if ($this->boardService->isArchived(null, $boardId)) {
			throw new StatusException('Operation not allowed. This board is archived.');
		}
		$stack = new Stack();
		$stack->setTitle($title);
		$stack->setBoardId($boardId);
		$stack->setOrder($order);
		return $this->stackMapper->insert($stack);

	}

	/**
	 * @param $id
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\DoesNotExistException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function delete($id) {


		if ( is_numeric($id) === false ) {
			throw new BadRequestException('stack id must be a number');
		}

		$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE);

		$stack = $this->stackMapper->find($id);
		$stack->setDeletedAt(time());
		$this->stackMapper->update($stack);

		$this->enrichStackWithCards($stack);

		return $stack;
	}

	/**
	 * @param $id
	 * @param $title
	 * @param $boardId
	 * @param $order
	 * @param $deletedAt
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws StatusException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\DoesNotExistException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function update($id, $title, $boardId, $order, $deletedAt) {

		if (is_numeric($id) === false) {
			throw new BadRequestException('stack id must be a number');
		}

		if ($title === false || $title === null) {
			throw new BadRequestException('title must be provided');
		}

		if (is_numeric($boardId) === false) {
			throw new BadRequestException('board id must be a number');
		}

		if (is_numeric($order) === false) {
			throw new BadRequestException('order must be a number');
		}

		$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE);
		if ($this->boardService->isArchived($this->stackMapper, $id)) {
			throw new StatusException('Operation not allowed. This board is archived.');
		}
		$stack = $this->stackMapper->find($id);
		$stack->setTitle($title);
		$stack->setBoardId($boardId);
		$stack->setOrder($order);
		$stack->setDeletedAt($deletedAt);
		return $this->stackMapper->update($stack);
	}

	/**
	 * @param $id
	 * @param $order
	 * @return array
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\DoesNotExistException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function reorder($id, $order) {

		if (is_numeric($id) === false) {
			throw new BadRquestException('id must be a number');
		}

		if ($order === false || $order === null) {
			throw new BadRequestException('order must be provided');
		}

		$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_EDIT);
		$stackToSort = $this->stackMapper->find($id);
		$stacks = $this->stackMapper->findAll($stackToSort->getBoardId());
		$result = [];
		$i = 0;
		foreach ($stacks as $stack) {
			if ($stack->id === $id) {
				$stack->setOrder($order);
			}

			if ($i === $order) {
				$i++;
			}

			if ($stack->id !== $id) {
				$stack->setOrder($i++);
			}
			$this->stackMapper->update($stack);
			$result[$stack->getOrder()] = $stack;
		}

		return $result;
	}
}


1		<?php
2		/**
3		* @copyright Copyright (c) 2016 Julius Härtl <[email protected]>
4		*
5		* @author Julius Härtl <[email protected]>
6		*
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\Deck\Service;
25
26		use OCA\Deck\Db\Acl;
27		use OCA\Deck\Db\CardMapper;
28		use OCA\Deck\Db\BoardMapper;
29		use OCA\Deck\Db\LabelMapper;
30		use OCA\Deck\Db\AssignedUsersMapper;
31		use OCA\Deck\Db\Stack;
32		use OCA\Deck\Db\StackMapper;
33		use OCA\Deck\StatusException;
34		use OCA\Deck\BadRequestException;
35
36
37		class StackService {
38
39		private $stackMapper;
40		private $cardMapper;
41		private $boardMapper;
42		private $labelMapper;
43		private $permissionService;
44		private $boardService;
45		private $cardService;
46		private $assignedUsersMapper;
47		private $attachmentService;
48
49	View Code Duplication	public function __construct(
		0 ignored issues – show Duplication introduced 2017-10-10 17:37 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...
50		StackMapper $stackMapper,
51		BoardMapper $boardMapper,
52		CardMapper $cardMapper,
53		LabelMapper $labelMapper,
54		PermissionService $permissionService,
55		BoardService $boardService,
56		CardService $cardService,
57		AssignedUsersMapper $assignedUsersMapper,
58		AttachmentService $attachmentService
59		) {
60		$this->stackMapper = $stackMapper;
61		$this->boardMapper = $boardMapper;
62		$this->cardMapper = $cardMapper;
63		$this->labelMapper = $labelMapper;
64		$this->permissionService = $permissionService;
65		$this->boardService = $boardService;
66		$this->cardService = $cardService;
67		$this->assignedUsersMapper = $assignedUsersMapper;
68		$this->attachmentService = $attachmentService;
69		}
70
71		private function enrichStackWithCards($stack) {
72		$cards = $this->cardMapper->findAll($stack->id);
73
74		if(is_null($cards)) {
75		return;
76		}
77
78		foreach ($cards as $card) {
79		$this->cardService->enrich($card);
80		}
81
82		$stack->setCards($cards);
83		}
84
85		private function enrichStacksWithCards($stacks) {
86		foreach ($stacks as $stack) {
87		$this->enrichStackWithCards($stack);
88		}
89		}
90
91		/**
92		* @param $stackId
93		* @return \OCP\AppFramework\Db\Entity
94		* @throws \OCP\AppFramework\Db\DoesNotExistException
95		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
96		* @throws BadRequestException
97		*/
98		public function find($stackId) {
99		if (is_numeric($stackId) === false) {
100		throw new BadRequestException('stack id must be a number');
101		}
102
103		$stack = $this->stackMapper->find($stackId);
104		$cards = $this->cardMapper->findAll($stackId);
105		foreach ($cards as $cardIndex => $card) {
106		$assignedUsers = $this->assignedUsersMapper->find($card->getId());
107		$card->setAssignedUsers($assignedUsers);
108		$card->setAttachmentCount($this->attachmentService->count($card->getId()));
109		}
110		$stack->setCards($cards);
111		return $stack;
112		}
113
114		/**
115		* @param $boardId
116		* @return array
117		* @throws \OCA\Deck\NoPermissionException
118		* @throws BadRequestException
119		*/
120		public function findAll($boardId) {
121		if (is_numeric($boardId) === false) {
122		throw new BadRequestException('boardId must be a number');
123		}
124
125		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
126		$stacks = $this->stackMapper->findAll($boardId);
127		$this->enrichStacksWithCards($stacks);
128		return $stacks;
129		}
130
131		public function fetchDeleted($boardId) {
132		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
133		$stacks = $this->stackMapper->findDeleted($boardId);
134		$this->enrichStacksWithCards($stacks);
135		return $stacks;
136		}
137
138		/**
139		* @param $boardId
140		* @return array
141		* @throws \OCA\Deck\NoPermissionException
142		* @throws BadRequestException
143		*/
144		public function findAllArchived($boardId) {
145
146		if (is_numeric($boardId) === false) {
147		throw new BadRequestException('board id must be a number');
148		}
149
150		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
151		$stacks = $this->stackMapper->findAll($boardId);
152		$labels = $this->labelMapper->getAssignedLabelsForBoard($boardId);
153		foreach ($stacks as $stackIndex => $stack) {
154		$cards = $this->cardMapper->findAllArchived($stack->id);
155		foreach ($cards as $cardIndex => $card) {
156		if (array_key_exists($card->id, $labels)) {
157		$cards[$cardIndex]->setLabels($labels[$card->id]);
158		}
159		}
160		$stacks[$stackIndex]->setCards($cards);
161		}
162		return $stacks;
163		}
164
165		/**
166		* @param $title
167		* @param $boardId
168		* @param integer $order
169		* @return \OCP\AppFramework\Db\Entity
170		* @throws StatusException
171		* @throws \OCA\Deck\NoPermissionException
172		* @throws \OCP\AppFramework\Db\DoesNotExistException
173		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
174		* @throws BadRequestException
175		*/
176	View Code Duplication	public function create($title, $boardId, $order) {
		0 ignored issues – show Duplication introduced 2017-08-01 00:15 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...
177
178		if ($title === false \|\| $title === null) {
179		throw new BadRequestException('title must be provided');
180		}
181
182		if (is_numeric($order) === false) {
183		throw new BadRequestException('order must be a number');
184		}
185
186		if (is_numeric($boardId) === false) {
187		throw new BadRequestException('board id must be a number');
188		}
189
190		$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE);
191		if ($this->boardService->isArchived(null, $boardId)) {
192		throw new StatusException('Operation not allowed. This board is archived.');
193		}
194		$stack = new Stack();
195		$stack->setTitle($title);
196		$stack->setBoardId($boardId);
197		$stack->setOrder($order);
198		return $this->stackMapper->insert($stack);
199
200		}
201
202		/**
203		* @param $id
204		* @return \OCP\AppFramework\Db\Entity
205		* @throws \OCA\Deck\NoPermissionException
206		* @throws \OCP\AppFramework\Db\DoesNotExistException
207		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
208		* @throws BadRequestException
209		*/
210	View Code Duplication	public function delete($id) {
		0 ignored issues – show Duplication introduced 2018-07-20 15: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...
211
212		if ( is_numeric($id) === false ) {
213		throw new BadRequestException('stack id must be a number');
214		}
215
216		$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE);
217
218		$stack = $this->stackMapper->find($id);
219		$stack->setDeletedAt(time());
220		$this->stackMapper->update($stack);
221
222		$this->enrichStackWithCards($stack);
223
224		return $stack;
225		}
226
227		/**
228		* @param $id
229		* @param $title
230		* @param $boardId
231		* @param $order
232		* @param $deletedAt
233		* @return \OCP\AppFramework\Db\Entity
234		* @throws StatusException
235		* @throws \OCA\Deck\NoPermissionException
236		* @throws \OCP\AppFramework\Db\DoesNotExistException
237		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
238		* @throws BadRequestException
239		*/
240		public function update($id, $title, $boardId, $order, $deletedAt) {
241
242		if (is_numeric($id) === false) {
243		throw new BadRequestException('stack id must be a number');
244		}
245
246		if ($title === false \|\| $title === null) {
247		throw new BadRequestException('title must be provided');
248		}
249
250		if (is_numeric($boardId) === false) {
251		throw new BadRequestException('board id must be a number');
252		}
253
254		if (is_numeric($order) === false) {
255		throw new BadRequestException('order must be a number');
256		}
257
258		$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE);
259		if ($this->boardService->isArchived($this->stackMapper, $id)) {
260		throw new StatusException('Operation not allowed. This board is archived.');
261		}
262		$stack = $this->stackMapper->find($id);
263		$stack->setTitle($title);
264		$stack->setBoardId($boardId);
265		$stack->setOrder($order);
266		$stack->setDeletedAt($deletedAt);
267		return $this->stackMapper->update($stack);
268		}
269
270		/**
271		* @param $id
272		* @param $order
273		* @return array
274		* @throws \OCA\Deck\NoPermissionException
275		* @throws \OCP\AppFramework\Db\DoesNotExistException
276		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
277		* @throws BadRequestException
278		*/
279		public function reorder($id, $order) {
280
281		if (is_numeric($id) === false) {
282		throw new BadRquestException('id must be a number');
283		}
284
285		if ($order === false \|\| $order === null) {
286		throw new BadRequestException('order must be provided');
287		}
288
289		$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_EDIT);
290		$stackToSort = $this->stackMapper->find($id);
291		$stacks = $this->stackMapper->findAll($stackToSort->getBoardId());
292		$result = [];
293		$i = 0;
294		foreach ($stacks as $stack) {
295		if ($stack->id === $id) {
296		$stack->setOrder($order);
297		}
298
299		if ($i === $order) {
300		$i++;
301		}
302
303		if ($stack->id !== $id) {
304		$stack->setOrder($i++);
305		}
306		$this->stackMapper->update($stack);
307		$result[$stack->getOrder()] = $stack;
308		}
309
310		return $result;
311		}
312		}
313

nextcloud / deck