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 CronExpression 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 CronExpression, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
10 | class CronExpression |
||
11 | { |
||
12 | /** |
||
13 | * Weekday look-up table |
||
14 | * |
||
15 | * @var array |
||
16 | */ |
||
17 | protected static $weekdays = [ |
||
18 | 'sun' => 0, |
||
19 | 'mon' => 1, |
||
20 | 'tue' => 2, |
||
21 | 'wed' => 3, |
||
22 | 'thu' => 4, |
||
23 | 'fri' => 5, |
||
24 | 'sat' => 6 |
||
25 | ]; |
||
26 | |||
27 | /** |
||
28 | * Month name look-up table |
||
29 | * |
||
30 | * @var array |
||
31 | */ |
||
32 | protected static $months = [ |
||
33 | 'jan' => 1, |
||
34 | 'feb' => 2, |
||
35 | 'mar' => 3, |
||
36 | 'apr' => 4, |
||
37 | 'may' => 5, |
||
38 | 'jun' => 6, |
||
39 | 'jul' => 7, |
||
40 | 'aug' => 8, |
||
41 | 'sep' => 9, |
||
42 | 'oct' => 10, |
||
43 | 'nov' => 11, |
||
44 | 'dec' => 12 |
||
45 | ]; |
||
46 | |||
47 | /** |
||
48 | * Value boundaries |
||
49 | * |
||
50 | * @var array |
||
51 | */ |
||
52 | protected static $boundaries = [ |
||
53 | 0 => [ |
||
54 | 'min' => 0, |
||
55 | 'max' => 59 |
||
56 | ], |
||
57 | 1 => [ |
||
58 | 'min' => 0, |
||
59 | 'max' => 23 |
||
60 | ], |
||
61 | 2 => [ |
||
62 | 'min' => 1, |
||
63 | 'max' => 31 |
||
64 | ], |
||
65 | 3 => [ |
||
66 | 'min' => 1, |
||
67 | 'max' => 12 |
||
68 | ], |
||
69 | 4 => [ |
||
70 | 'min' => 0, |
||
71 | 'max' => 7 |
||
72 | ] |
||
73 | ]; |
||
74 | |||
75 | /** |
||
76 | * Cron expression |
||
77 | * |
||
78 | * @var string |
||
79 | */ |
||
80 | protected $expression; |
||
81 | |||
82 | /** |
||
83 | * Time zone |
||
84 | * |
||
85 | * @var \DateTimeZone |
||
86 | */ |
||
87 | protected $timeZone; |
||
88 | |||
89 | /** |
||
90 | * Matching register |
||
91 | * |
||
92 | * @var array|null |
||
93 | */ |
||
94 | protected $register; |
||
95 | |||
96 | /** |
||
97 | * Class constructor sets cron expression property |
||
98 | * |
||
99 | * @param string $expression cron expression |
||
100 | * @param \DateTimeZone|null $timeZone |
||
101 | */ |
||
102 | 90 | public function __construct(string $expression = '* * * * *', \DateTimeZone $timeZone = null) |
|
107 | |||
108 | /** |
||
109 | * Set expression |
||
110 | * |
||
111 | * @param string $expression |
||
112 | * @return self |
||
113 | */ |
||
114 | 90 | public function setExpression(string $expression): self |
|
121 | |||
122 | /** |
||
123 | * Set time zone |
||
124 | * |
||
125 | * @param \DateTimeZone|null $timeZone |
||
126 | * @return self |
||
127 | */ |
||
128 | 90 | public function setTimeZone(\DateTimeZone $timeZone = null): self |
|
133 | |||
134 | /** |
||
135 | * Calculate next matching timestamp |
||
136 | * |
||
137 | * @param mixed $start either a \DateTime object, a timestamp or null for current date/time |
||
138 | * @return int|bool next matching timestamp, or false on error |
||
139 | */ |
||
140 | 8 | public function getNext($start = null) |
|
171 | |||
172 | /** |
||
173 | * @param \DateTime $now |
||
174 | * @param array $pointer |
||
175 | * @return array |
||
176 | */ |
||
177 | 8 | private function adjust(\DateTime $now, array &$pointer): array |
|
212 | |||
213 | /** |
||
214 | * @param \DateTime $now |
||
215 | * @param array $current |
||
216 | * @return bool |
||
217 | */ |
||
218 | 8 | private function forward(\DateTime $now, array $current): bool |
|
238 | |||
239 | /** |
||
240 | * Parse and validate cron expression |
||
241 | * |
||
242 | * @return bool true if expression is valid, or false on error |
||
243 | */ |
||
244 | 89 | public function isValid(): bool |
|
258 | |||
259 | /** |
||
260 | * Match current or given date/time against cron expression |
||
261 | * |
||
262 | * @param mixed $now \DateTime object, timestamp or null |
||
263 | * @return bool |
||
264 | */ |
||
265 | 90 | public function isMatching($now = null): bool |
|
283 | |||
284 | /** |
||
285 | * @param array $segments |
||
286 | * @return bool |
||
287 | * @throws \Exception |
||
288 | */ |
||
289 | 90 | private function match(array $segments): bool |
|
302 | |||
303 | /** |
||
304 | * Parse whole cron expression |
||
305 | * |
||
306 | * @return array |
||
307 | * @throws \Exception |
||
308 | */ |
||
309 | 90 | private function parse(): array |
|
327 | |||
328 | /** |
||
329 | * Parse one segment of a cron expression |
||
330 | * |
||
331 | * @param int $index |
||
332 | * @param string $segment |
||
333 | * @param array $register |
||
334 | * @throws \Exception |
||
335 | */ |
||
336 | 86 | private function parseSegment($index, array &$register, $segment) |
|
351 | |||
352 | /** |
||
353 | * @param int $index |
||
354 | * @param array $register |
||
355 | * @param string $element |
||
356 | * @throws \Exception |
||
357 | */ |
||
358 | 86 | private function parseElement(int $index, array &$register, string $element) |
|
378 | |||
379 | /** |
||
380 | * Parse range of values, e.g. "5-10" |
||
381 | * |
||
382 | * @param int $index |
||
383 | * @param array $register |
||
384 | * @param string $range |
||
385 | * @param int $stepping |
||
386 | * @throws \Exception |
||
387 | */ |
||
388 | 79 | private function parseRange(int $index, array &$register, string $range, int $stepping) |
|
400 | |||
401 | /** |
||
402 | * Parse stepping notation, e.g. "5-10/2" => 2 |
||
403 | * |
||
404 | * @param int $index |
||
405 | * @param string $element |
||
406 | * @param int $stepping |
||
407 | * @throws \Exception |
||
408 | */ |
||
409 | 42 | private function parseStepping(int $index, string &$element, int &$stepping) |
|
418 | |||
419 | /** |
||
420 | * Validate whether a given range of values exceeds allowed value boundaries |
||
421 | * |
||
422 | * @param int $index |
||
423 | * @param array $range |
||
424 | * @return array |
||
425 | * @throws \Exception |
||
426 | */ |
||
427 | 44 | private function validateRange(int $index, array $range): array |
|
439 | /** |
||
440 | * @param int $index |
||
441 | * @param string $value |
||
442 | * @throws \Exception |
||
443 | */ |
||
444 | 65 | private function validateValue(int $index, string $value) |
|
455 | |||
456 | /** |
||
457 | * @param int $index |
||
458 | * @param array $segments |
||
459 | * @throws \Exception |
||
460 | */ |
||
461 | 42 | private function validateStepping(int $index, array $segments) |
|
471 | |||
472 | /** |
||
473 | * @param int $index |
||
474 | * @param array $register |
||
475 | * @param array $range |
||
476 | * @param int $stepping |
||
477 | */ |
||
478 | 74 | private function fillRegister(int $index, array &$register, array $range, int $stepping) |
|
490 | |||
491 | /** |
||
492 | * @param int $index |
||
493 | * @param array $register |
||
494 | * @param array $range |
||
495 | * @param int $value |
||
496 | */ |
||
497 | 29 | private function fillRegisterAcrossBoundaries(int $index, array &$register, array $range, int $value) |
|
503 | |||
504 | /** |
||
505 | * @param int $index |
||
506 | * @param array $register |
||
507 | * @param array $range |
||
508 | * @param int $value |
||
509 | */ |
||
510 | 73 | private function fillRegisterBetweenBoundaries(int $index, array &$register, array $range, int $value) |
|
516 | } |
||
517 |
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.