Passed
Pull Request — master (#929)
by René
04:55
created

OptionController::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 19
Code Lines 8

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 0
CRAP Score 2

Importance

Changes 1
Bugs 0 Features 0
Metric Value
cc 1
eloc 8
nc 1
nop 9
dl 0
loc 19
ccs 0
cts 19
cp 0
crap 2
rs 10
c 1
b 0
f 0

How to fix   Many Parameters   

Many Parameters

Methods with many parameters are not only hard to understand, but their parameters also often become inconsistent when you need more, or different data.

There are several approaches to avoid long parameter lists:

1
<?php
2
/**
3
 * @copyright Copyright (c) 2017 Vinzenz Rosenkranz <[email protected]>
4
 *
5
 * @author René Gieling <[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\Polls\Controller;
25
26
use Exception;
27
use OCP\AppFramework\Db\DoesNotExistException;
28
29
use OCP\IRequest;
30
use OCP\ILogger;
31
use OCP\AppFramework\Controller;
32
use OCP\AppFramework\Http;
33
use OCP\AppFramework\Http\DataResponse;
34
35
use OCP\IGroupManager;
36
use OCP\Security\ISecureRandom;
37
38
use OCA\Polls\Db\Poll;
39
use OCA\Polls\Db\PollMapper;
40
use OCA\Polls\Db\Option;
41
use OCA\Polls\Db\OptionMapper;
42
use OCA\Polls\Service\LogService;
43
use OCA\Polls\Model\Acl;
44
45
class OptionController extends Controller {
46
47
	private $userId;
48
	private $optionMapper;
49
50
	private $groupManager;
51
	private $pollMapper;
52
	private $logger;
53
	private $logService;
54
	private $acl;
55
56
	/**
57
	 * OptionController constructor.
58
	 * @param string $appName
59
	 * @param $UserId
60
	 * @param IRequest $request
61
	 * @param ILogger $logger
62
	 * @param OptionMapper $optionMapper
63
	 * @param IGroupManager $groupManager
64
	 * @param PollMapper $pollMapper
65
	 * @param LogService $logService
66
	 * @param Acl $acl
67
	 */
68
69
	public function __construct(
70
		string $appName,
71
		$UserId,
72
		IRequest $request,
73
		OptionMapper $optionMapper,
74
		IGroupManager $groupManager,
75
		PollMapper $pollMapper,
76
		ILogger $logger,
77
		LogService $logService,
78
		Acl $acl
79
	) {
80
		parent::__construct($appName, $request);
81
		$this->userId = $UserId;
82
		$this->optionMapper = $optionMapper;
83
		$this->groupManager = $groupManager;
84
		$this->pollMapper = $pollMapper;
85
		$this->logger = $logger;
86
		$this->logService = $logService;
87
		$this->acl = $acl;
88
	}
89
90
91
	/**
92
	 * Get all options of given poll
93
	 * @NoAdminRequired
94
	 * @param integer $pollId
95
	 * @return array Array of Option objects
96
	 */
97
	public function get($pollId) {
98
99
		try {
100
101
			if (!$this->acl->getFoundByToken()) {
102
				$this->acl->setPollId($pollId);
103
			}
104
105
			return new DataResponse($this->optionMapper->findByPoll($pollId), Http::STATUS_OK);
106
107
		} catch (DoesNotExistException $e) {
108
			return new DataResponse($e, Http::STATUS_NOT_FOUND);
109
		}
110
	}
111
112
113
	/**
114
	 * getByToken
115
	 * Read all options of a poll based on a share token and return list as array
116
	 * @NoAdminRequired
117
	 * @PublicPage
118
	 * @NoCSRFRequired
119
	 * @param string $token
120
	 * @return DataResponse
121
	 */
122
	public function getByToken($token) {
123
124
		try {
125
			$this->acl->setToken($token);
126
			return $this->get($this->acl->getPollId());
127
128
		} catch (DoesNotExistException $e) {
129
			return new DataResponse($e, Http::STATUS_NOT_FOUND);
130
		}
131
	}
132
133
	/**
134
	 * Add a new Option to poll
135
	 * @NoAdminRequired
136
	 * @param Option $option
137
	 * @return DataResponse
138
	 */
139
	public function add($option) {
140
141
		try {
142
143
			if (!$this->acl->setPollId($option['pollId'])->getAllowEdit()) {
144
				return new DataResponse(null, Http::STATUS_UNAUTHORIZED);
145
			}
146
147
			$NewOption = new Option();
148
149
			$NewOption->setPollId($option['pollId']);
150
			$NewOption->setPollOptionText(trim(htmlspecialchars($option['pollOptionText'])));
151
			$NewOption->setTimestamp($option['timestamp']);
152
153
			if ($option['timestamp'] === 0) {
154
				$NewOption->setOrder($option['order']);
0 ignored issues
show
Bug introduced by
The call to OCA\Polls\Db\Option::setOrder() has too few arguments starting with order. ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-call  annotation

154
				$NewOption->/** @scrutinizer ignore-call */ 
155
                setOrder($option['order']);

This check compares calls to functions or methods with their respective definitions. If the call has less arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. Please note the @ignore annotation hint above.

Loading history...
155
			} else {
156
				$NewOption->setOrder($option['timestamp']);
157
			}
158
159
			$this->optionMapper->insert($NewOption);
160
			$this->logService->setLog($option['pollId'], 'addOption');
161
			return new DataResponse($NewOption, Http::STATUS_OK);
162
163
		} catch (Exception $e) {
164
			return new DataResponse($e, Http::STATUS_NOT_FOUND);
165
		}
166
167
	}
168
169
	/**
170
	 * Update poll option
171
	 * @NoAdminRequired
172
	 * @param Option $option
173
	 * @return DataResponse
174
	 */
175
	public function update($option) {
176
177
		try {
178
			$this->logger->alert(json_encode($option));
179
			$updateOption = $this->optionMapper->find($option['id']);
180
181
			if (!$this->acl->setPollId($option['pollId'])->getAllowEdit()) {
182
				return new DataResponse(null, Http::STATUS_UNAUTHORIZED);
183
			}
184
185
			$updateOption->setPollOptionText(trim(htmlspecialchars($option['pollOptionText'])));
186
			$updateOption->setTimestamp($option['timestamp']);
187
188
			if ($option['timestamp'] === 0) {
189
				$updateOption->setOrder($option['order']);
0 ignored issues
show
Bug introduced by
The call to OCA\Polls\Db\Option::setOrder() has too few arguments starting with order. ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-call  annotation

189
				$updateOption->/** @scrutinizer ignore-call */ 
190
                   setOrder($option['order']);

This check compares calls to functions or methods with their respective definitions. If the call has less arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. Please note the @ignore annotation hint above.

Loading history...
190
			} else {
191
				$updateOption->setOrder($option['timestamp']);
192
			}
193
194
			if ($option['confirmed']) {
195
				// do not update confirmation date, if option is already confirmed
196
				if (!$updateOption->setConfirmed()){
0 ignored issues
show
Bug introduced by
The call to OCA\Polls\Db\Option::setConfirmed() has too few arguments starting with value. ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-call  annotation

196
				if (!$updateOption->/** @scrutinizer ignore-call */ setConfirmed()){

This check compares calls to functions or methods with their respective definitions. If the call has less arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. Please note the @ignore annotation hint above.

Loading history...
Bug introduced by
Are you sure the usage of $updateOption->setConfirmed() targeting OCA\Polls\Db\Option::setConfirmed() seems to always return null.

This check looks for function or method calls that always return null and whose return value is used.

class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
if ($a->getObject()) {

The method getObject() can return nothing but null, so it makes no sense to use the return value.

The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.

Loading history...
197
					$updateOption->setConfirmed(time());
198
				}
199
200
			} else {
201
				$updateOption->setConfirmed(0);
202
			}
203
204
			$this->optionMapper->update($updateOption);
205
			$this->logService->setLog($option['pollId'], 'updateOption');
206
207
			return new DataResponse($updateOption, Http::STATUS_OK);
208
209
		} catch (Exception $e) {
210
			return new DataResponse($e, Http::STATUS_NOT_FOUND);
211
		}
212
	}
213
214
	/**
215
	 * Remove a single option
216
	 * @NoAdminRequired
217
	 * @param Option $option
218
	 * @return DataResponse
219
	 */
220
	public function remove($option) {
221
		try {
222
223
			if (!$this->acl->setPollId($option['pollId'])->getAllowEdit()) {
224
				return new DataResponse(null, Http::STATUS_UNAUTHORIZED);
225
			}
226
227
			$this->optionMapper->remove($option['id']);
228
			$this->logService->setLog($option['pollId'], 'deleteOption');
229
230
			return new DataResponse(array(
231
				'action' => 'deleted',
232
				'optionId' => $option['id']
233
			), Http::STATUS_OK);
234
235
		} catch (Exception $e) {
236
			return new DataResponse($e, Http::STATUS_NOT_FOUND);
237
		}
238
239
	}
240
241
}
242