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:
Complex classes like Job 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 Job, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
37 | class Job |
||
38 | { |
||
39 | /** State if job is inserted, but not yet ready to be started. */ |
||
40 | const STATE_NEW = 'new'; |
||
41 | |||
42 | /** |
||
43 | * State if job is inserted, and might be started. |
||
44 | * |
||
45 | * It is important to note that this does not automatically mean that all |
||
46 | * jobs of this state can actually be started, but you have to check |
||
47 | * isStartable() to be absolutely sure. |
||
48 | * |
||
49 | * In contrast to NEW, jobs of this state at least might be started, |
||
50 | * while jobs of state NEW never are allowed to be started. |
||
51 | */ |
||
52 | const STATE_PENDING = 'pending'; |
||
53 | |||
54 | /** State if job was never started, and will never be started. */ |
||
55 | const STATE_CANCELED = 'canceled'; |
||
56 | |||
57 | /** State if job was started and has not exited, yet. */ |
||
58 | const STATE_RUNNING = 'running'; |
||
59 | |||
60 | /** State if job exists with a successful exit code. */ |
||
61 | const STATE_FINISHED = 'finished'; |
||
62 | |||
63 | /** State if job exits with a non-successful exit code. */ |
||
64 | const STATE_FAILED = 'failed'; |
||
65 | |||
66 | /** State if job exceeds its configured maximum runtime. */ |
||
67 | const STATE_TERMINATED = 'terminated'; |
||
68 | |||
69 | /** |
||
70 | * State if an error occurs in the runner command. |
||
71 | * |
||
72 | * The runner command is the command that actually launches the individual |
||
73 | * jobs. If instead an error occurs in the job command, this will result |
||
74 | * in a state of FAILED. |
||
75 | */ |
||
76 | const STATE_INCOMPLETE = 'incomplete'; |
||
77 | |||
78 | const DEFAULT_QUEUE = 'default'; |
||
79 | const MAX_QUEUE_LENGTH = 50; |
||
80 | |||
81 | const PRIORITY_LOW = -5; |
||
82 | const PRIORITY_DEFAULT = 0; |
||
83 | const PRIORITY_HIGH = 5; |
||
84 | |||
85 | /** @ORM\Id @ORM\GeneratedValue(strategy = "AUTO") @ORM\Column(type = "bigint", options = {"unsigned": true}) */ |
||
86 | private $id; |
||
87 | |||
88 | /** @ORM\Column(type = "string", length = 15) */ |
||
89 | private $state; |
||
90 | |||
91 | /** @ORM\Column(type = "string", length = Job::MAX_QUEUE_LENGTH) */ |
||
92 | private $queue; |
||
93 | |||
94 | /** @ORM\Column(type = "smallint") */ |
||
95 | private $priority = 0; |
||
96 | |||
97 | /** @ORM\Column(type = "datetime", name="createdAt") */ |
||
98 | private $createdAt; |
||
99 | |||
100 | /** @ORM\Column(type = "datetime", name="startedAt", nullable = true) */ |
||
101 | private $startedAt; |
||
102 | |||
103 | /** @ORM\Column(type = "datetime", name="checkedAt", nullable = true) */ |
||
104 | private $checkedAt; |
||
105 | |||
106 | /** @ORM\Column(type = "string", name="workerName", length = 50, nullable = true) */ |
||
107 | private $workerName; |
||
108 | |||
109 | /** @ORM\Column(type = "datetime", name="executeAfter", nullable = true) */ |
||
110 | private $executeAfter; |
||
111 | |||
112 | /** @ORM\Column(type = "datetime", name="closedAt", nullable = true) */ |
||
113 | private $closedAt; |
||
114 | |||
115 | /** @ORM\Column(type = "string") */ |
||
116 | private $command; |
||
117 | |||
118 | /** @ORM\Column(type = "json") */ |
||
119 | private $args = []; |
||
120 | |||
121 | /** |
||
122 | * @ORM\ManyToMany(targetEntity = "Job", fetch = "EAGER") |
||
123 | * @ORM\JoinTable(name="jms_job_dependencies", |
||
124 | * joinColumns = { @ORM\JoinColumn(name = "source_job_id", referencedColumnName = "id") }, |
||
125 | * inverseJoinColumns = { @ORM\JoinColumn(name = "dest_job_id", referencedColumnName = "id")} |
||
126 | * ) |
||
127 | */ |
||
128 | private $dependencies; |
||
129 | |||
130 | /** @ORM\Column(type = "text", nullable = true) */ |
||
131 | private $output; |
||
132 | |||
133 | /** @ORM\Column(type = "text", name="errorOutput", nullable = true) */ |
||
134 | private $errorOutput; |
||
135 | |||
136 | /** @ORM\Column(type = "smallint", name="exitCode", nullable = true, options = {"unsigned": true}) */ |
||
137 | private $exitCode; |
||
138 | |||
139 | /** @ORM\Column(type = "smallint", name="maxRuntime", options = {"unsigned": true}) */ |
||
140 | private $maxRuntime = 0; |
||
141 | |||
142 | /** @ORM\Column(type = "smallint", name="maxRetries", options = {"unsigned": true}) */ |
||
143 | private $maxRetries = 0; |
||
144 | |||
145 | /** |
||
146 | * @ORM\ManyToOne(targetEntity = "Job", inversedBy = "retryJobs") |
||
147 | * @ORM\JoinColumn(name="originalJob_id", referencedColumnName="id") |
||
148 | */ |
||
149 | private $originalJob; |
||
150 | |||
151 | /** @ORM\OneToMany(targetEntity = "Job", mappedBy = "originalJob", cascade = {"persist", "remove", "detach", "refresh"}) */ |
||
152 | private $retryJobs; |
||
153 | |||
154 | /** @ORM\Column(type = "jms_job_safe_object", name="stackTrace", nullable = true) */ |
||
155 | private $stackTrace; |
||
156 | |||
157 | /** @ORM\Column(type = "smallint", nullable = true, options = {"unsigned": true}) */ |
||
158 | private $runtime; |
||
159 | |||
160 | /** @ORM\Column(type = "integer", name="memoryUsage", nullable = true, options = {"unsigned": true}) */ |
||
161 | private $memoryUsage; |
||
162 | |||
163 | /** @ORM\Column(type = "integer", name="memoryUsageReal", nullable = true, options = {"unsigned": true}) */ |
||
164 | private $memoryUsageReal; |
||
165 | |||
166 | /** |
||
167 | * This may store any entities which are related to this job, and are |
||
168 | * managed by Doctrine. |
||
169 | * |
||
170 | * It is effectively a many-to-any association. |
||
171 | */ |
||
172 | private $relatedEntities; |
||
173 | |||
174 | public static function create($command, array $args = array(), $confirmed = true, $queue = self::DEFAULT_QUEUE, $priority = self::PRIORITY_DEFAULT) |
||
178 | |||
179 | 30 | public static function isNonSuccessfulFinalState($state) |
|
183 | |||
184 | public static function getStates() |
||
197 | |||
198 | 74 | public function __construct($command, array $args = array(), $confirmed = true, $queue = self::DEFAULT_QUEUE, $priority = self::PRIORITY_DEFAULT) |
|
219 | |||
220 | 8 | public function __clone() |
|
237 | |||
238 | 44 | public function getId() |
|
242 | |||
243 | 36 | public function getState() |
|
247 | |||
248 | 28 | public function setWorkerName($workerName) |
|
254 | |||
255 | 2 | public function getWorkerName() |
|
259 | |||
260 | 4 | public function getPriority() |
|
264 | |||
265 | 30 | public function isInFinalState() |
|
269 | |||
270 | 28 | public function isStartable() |
|
280 | |||
281 | 50 | public function setState($newState) |
|
336 | |||
337 | 2 | public function getCreatedAt() |
|
341 | |||
342 | public function getClosedAt() |
||
346 | |||
347 | public function getExecuteAfter() |
||
351 | |||
352 | 4 | public function setExecuteAfter(\DateTime $executeAfter) |
|
358 | |||
359 | 28 | public function getCommand() |
|
363 | |||
364 | 28 | public function getArgs() |
|
368 | |||
369 | 2 | public function getRelatedEntities() |
|
373 | |||
374 | 30 | public function isClosedNonSuccessful() |
|
378 | |||
379 | 2 | public function findRelatedEntity($class) |
|
389 | |||
390 | 4 | public function addRelatedEntity($entity) |
|
404 | |||
405 | 6 | public function getDependencies() |
|
409 | |||
410 | 2 | public function hasDependency(Job $job) |
|
414 | |||
415 | 20 | public function addDependency(Job $job) |
|
429 | |||
430 | public function getRuntime() |
||
434 | |||
435 | 22 | public function setRuntime($time) |
|
441 | |||
442 | 2 | public function getMemoryUsage() |
|
446 | |||
447 | 2 | public function getMemoryUsageReal() |
|
451 | |||
452 | 6 | public function addOutput($output) |
|
458 | |||
459 | 6 | public function addErrorOutput($output) |
|
465 | |||
466 | 24 | public function setOutput($output) |
|
472 | |||
473 | 24 | public function setErrorOutput($output) |
|
479 | |||
480 | 6 | public function getOutput() |
|
484 | |||
485 | 6 | public function getErrorOutput() |
|
489 | |||
490 | 22 | public function setExitCode($code) |
|
496 | |||
497 | 2 | public function getExitCode() |
|
501 | |||
502 | 6 | public function setMaxRuntime($time) |
|
508 | |||
509 | 26 | public function getMaxRuntime() |
|
513 | |||
514 | 28 | public function getStartedAt() |
|
518 | |||
519 | public function getMaxRetries() |
||
523 | |||
524 | 8 | public function setMaxRetries($tries) |
|
530 | |||
531 | 14 | public function isRetryAllowed() |
|
541 | |||
542 | 6 | public function getOriginalJob() |
|
550 | |||
551 | 8 | public function setOriginalJob(Job $job) |
|
565 | |||
566 | 8 | public function addRetryJob(Job $job) |
|
577 | |||
578 | 36 | public function getRetryJobs() |
|
582 | |||
583 | 32 | public function isRetryJob() |
|
587 | |||
588 | public function isRetried() |
||
600 | |||
601 | 6 | public function checked() |
|
605 | |||
606 | 2 | public function getCheckedAt() |
|
610 | |||
611 | public function setStackTrace(FlattenException $ex) |
||
617 | |||
618 | 2 | public function getStackTrace() |
|
622 | |||
623 | 28 | public function getQueue() |
|
627 | |||
628 | 30 | public function isNew() |
|
632 | |||
633 | 30 | public function isPending() |
|
637 | |||
638 | public function isCanceled() |
||
642 | |||
643 | 28 | public function isRunning() |
|
647 | |||
648 | public function isTerminated() |
||
652 | |||
653 | public function isFailed() |
||
657 | |||
658 | public function isFinished() |
||
662 | |||
663 | public function isIncomplete() |
||
667 | |||
668 | 24 | public function __toString() |
|
672 | |||
673 | 20 | private function mightHaveStarted() |
|
689 | } |
||
690 |
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.