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:
| 1 | <?php |
||
| 37 | class JobList implements IJobList { |
||
| 38 | |||
| 39 | /** @var IDBConnection */ |
||
| 40 | protected $connection; |
||
| 41 | |||
| 42 | /**@var IConfig */ |
||
| 43 | protected $config; |
||
| 44 | |||
| 45 | /**@var ITimeFactory */ |
||
| 46 | protected $timeFactory; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @param IDBConnection $connection |
||
| 50 | * @param IConfig $config |
||
| 51 | * @param ITimeFactory $timeFactory |
||
| 52 | */ |
||
| 53 | public function __construct(IDBConnection $connection, IConfig $config, ITimeFactory $timeFactory) { |
||
| 58 | |||
| 59 | /** |
||
| 60 | * @param IJob|string $job |
||
| 61 | * @param mixed $argument |
||
| 62 | */ |
||
| 63 | public function add($job, $argument = null) { |
||
| 87 | |||
| 88 | /** |
||
| 89 | * @param IJob|string $job |
||
| 90 | * @param mixed $argument |
||
| 91 | */ |
||
| 92 | public function remove($job, $argument = null) { |
||
| 108 | |||
| 109 | /** |
||
| 110 | * @param int $id |
||
| 111 | */ |
||
| 112 | protected function removeById($id) { |
||
| 118 | |||
| 119 | /** |
||
| 120 | * check if a job is in the list |
||
| 121 | * |
||
| 122 | * @param IJob|string $job |
||
| 123 | * @param mixed $argument |
||
| 124 | * @return bool |
||
| 125 | */ |
||
| 126 | public function has($job, $argument) { |
||
| 147 | |||
| 148 | /** |
||
| 149 | * get all jobs in the list |
||
| 150 | * |
||
| 151 | * @return IJob[] |
||
| 152 | * @deprecated 9.0.0 - This method is dangerous since it can cause load and |
||
| 153 | * memory problems when creating too many instances. |
||
| 154 | */ |
||
| 155 | public function getAll() { |
||
| 172 | |||
| 173 | /** |
||
| 174 | * get the next job in the list |
||
| 175 | * |
||
| 176 | * @return IJob|null |
||
| 177 | */ |
||
| 178 | public function getNext() { |
||
| 179 | $query = $this->connection->getQueryBuilder(); |
||
| 180 | $query->select('*') |
||
| 181 | ->from('jobs') |
||
| 182 | ->where($query->expr()->lte('reserved_at', $query->createNamedParameter($this->timeFactory->getTime() - 12 * 3600, IQueryBuilder::PARAM_INT))) |
||
| 183 | ->orderBy('last_checked', 'ASC') |
||
| 184 | ->setMaxResults(1); |
||
| 185 | |||
| 186 | $update = $this->connection->getQueryBuilder(); |
||
| 187 | $update->update('jobs') |
||
| 188 | ->set('reserved_at', $update->createNamedParameter($this->timeFactory->getTime())) |
||
| 189 | ->set('last_checked', $update->createNamedParameter($this->timeFactory->getTime())) |
||
| 190 | ->where($update->expr()->eq('id', $update->createParameter('jobid'))) |
||
| 191 | ->andWhere($update->expr()->eq('reserved_at', $update->createParameter('reserved_at'))) |
||
| 192 | ->andWhere($update->expr()->eq('last_checked', $update->createParameter('last_checked'))); |
||
| 193 | |||
| 194 | $result = $query->execute(); |
||
| 195 | $row = $result->fetch(); |
||
| 196 | $result->closeCursor(); |
||
| 197 | |||
| 198 | if ($row) { |
||
| 199 | $update->setParameter('jobid', $row['id']); |
||
| 200 | $update->setParameter('reserved_at', $row['reserved_at']); |
||
| 201 | $update->setParameter('last_checked', $row['last_checked']); |
||
| 202 | $count = $update->execute(); |
||
| 203 | |||
| 204 | if ($count === 0) { |
||
| 205 | // Background job already executed elsewhere, try again. |
||
| 206 | return $this->getNext(); |
||
| 207 | } |
||
| 208 | $job = $this->buildJob($row); |
||
| 209 | |||
| 210 | if ($job === null) { |
||
| 211 | // Background job from disabled app, try again. |
||
| 212 | return $this->getNext(); |
||
| 213 | } |
||
| 214 | |||
| 215 | return $job; |
||
| 216 | } else { |
||
| 217 | return null; |
||
| 218 | } |
||
| 219 | } |
||
| 220 | |||
| 221 | /** |
||
| 222 | * @param int $id |
||
| 223 | * @return IJob|null |
||
| 224 | */ |
||
| 225 | View Code Duplication | public function getById($id) { |
|
| 240 | |||
| 241 | /** |
||
| 242 | * get the job object from a row in the db |
||
| 243 | * |
||
| 244 | * @param array $row |
||
| 245 | * @return IJob|null |
||
| 246 | */ |
||
| 247 | private function buildJob($row) { |
||
| 272 | |||
| 273 | /** |
||
| 274 | * set the job that was last ran |
||
| 275 | * |
||
| 276 | * @param IJob $job |
||
| 277 | */ |
||
| 278 | public function setLastJob(IJob $job) { |
||
| 282 | |||
| 283 | /** |
||
| 284 | * Remove the reservation for a job |
||
| 285 | * |
||
| 286 | * @param IJob $job |
||
| 287 | */ |
||
| 288 | View Code Duplication | public function unlockJob(IJob $job) { |
|
| 295 | |||
| 296 | /** |
||
| 297 | * get the id of the last ran job |
||
| 298 | * |
||
| 299 | * @return int |
||
| 300 | * @deprecated 9.1.0 - The functionality behind the value is deprecated, it |
||
| 301 | * only tells you which job finished last, but since we now allow multiple |
||
| 302 | * executors to run in parallel, it's not used to calculate the next job. |
||
| 303 | */ |
||
| 304 | public function getLastJob() { |
||
| 307 | |||
| 308 | /** |
||
| 309 | * set the lastRun of $job to now |
||
| 310 | * |
||
| 311 | * @param IJob $job |
||
| 312 | */ |
||
| 313 | public function setLastRun(IJob $job) { |
||
| 320 | |||
| 321 | /** |
||
| 322 | * @param IJob $job |
||
| 323 | * @param $timeTaken |
||
| 324 | */ |
||
| 325 | public function setExecutionTime(IJob $job, $timeTaken) { |
||
| 332 | } |
||
| 333 |
This check looks for function or method calls that always return null and whose return value is assigned to a variable.
The method
getObject()can return nothing but null, so it makes no sense to assign that value to a variable.The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.