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 Event 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 Event, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
12 | class Event |
||
13 | { |
||
14 | /** |
||
15 | * The command to run. |
||
16 | * @var string |
||
17 | */ |
||
18 | protected $command; |
||
19 | |||
20 | /** |
||
21 | * The working directory. |
||
22 | * @var string |
||
23 | */ |
||
24 | protected $cwd; |
||
25 | |||
26 | /** |
||
27 | * The user the command should run as. |
||
28 | * @var string |
||
29 | */ |
||
30 | protected $user; |
||
31 | |||
32 | /** |
||
33 | * The cron expression representing the event's frequency. |
||
34 | * @var string |
||
35 | */ |
||
36 | protected $expression = '* * * * * *'; |
||
37 | |||
38 | /** |
||
39 | * The timezone the date should be evaluated on. |
||
40 | * @var \DateTimeZone|string |
||
41 | */ |
||
42 | protected $timezone; |
||
43 | |||
44 | /** |
||
45 | * Indicates if the command should not overlap itself. |
||
46 | * @var bool |
||
47 | */ |
||
48 | protected $mutuallyExclusive = false; |
||
49 | |||
50 | /** |
||
51 | * Indicates if the command should run in background. |
||
52 | * @var bool |
||
53 | */ |
||
54 | protected $runInBackground = true; |
||
55 | |||
56 | /** |
||
57 | * The array of filter callbacks. These must return true |
||
58 | * @var array |
||
59 | */ |
||
60 | protected $filters = []; |
||
61 | |||
62 | /** |
||
63 | * The array of reject callbacks. |
||
64 | * @var array |
||
65 | */ |
||
66 | protected $rejects = []; |
||
67 | |||
68 | /** |
||
69 | * The location that output should be sent to. |
||
70 | * @var string |
||
71 | */ |
||
72 | protected $output = '/dev/null'; |
||
73 | |||
74 | /** |
||
75 | * The location of error output |
||
76 | * @var string |
||
77 | */ |
||
78 | protected $errorOutput = '/dev/null'; |
||
79 | |||
80 | /** |
||
81 | * Indicates whether output should be appended or added (> vs >>) |
||
82 | * @var bool |
||
83 | */ |
||
84 | protected $shouldAppendOutput = true; |
||
85 | |||
86 | /** |
||
87 | * The array of callbacks to be run before the event is started. |
||
88 | * @var array |
||
89 | */ |
||
90 | protected $beforeCallbacks = []; |
||
91 | |||
92 | /** |
||
93 | * The array of callbacks to be run after the event is finished. |
||
94 | * @var array |
||
95 | */ |
||
96 | protected $afterCallbacks = []; |
||
97 | |||
98 | /** |
||
99 | * The human readable description of the event. |
||
100 | * @var string |
||
101 | */ |
||
102 | public $description; |
||
103 | |||
104 | public function __construct(/* string */ $command) |
||
110 | |||
111 | /** |
||
112 | * @return string |
||
113 | */ |
||
114 | public function getCommand() |
||
118 | |||
119 | /** |
||
120 | * @return string |
||
121 | */ |
||
122 | public function __toString() |
||
126 | |||
127 | /** |
||
128 | * Build the command string. |
||
129 | * |
||
130 | * @return string |
||
131 | */ |
||
132 | public function compileCommand() |
||
161 | |||
162 | /** |
||
163 | * Run the given event. |
||
164 | * |
||
165 | * @return Process |
||
166 | */ |
||
167 | public function run() |
||
185 | |||
186 | /** |
||
187 | * Run the command in the foreground. |
||
188 | * |
||
189 | * @return Process |
||
190 | */ |
||
191 | View Code Duplication | protected function runCommandInForeground() |
|
198 | |||
199 | /** |
||
200 | * Run the command in the background. |
||
201 | * |
||
202 | * @return Process |
||
203 | */ |
||
204 | View Code Duplication | protected function runCommandInBackground() |
|
211 | |||
212 | /** |
||
213 | * Get the mutex path for managing concurrency |
||
214 | * |
||
215 | * @return string |
||
216 | */ |
||
217 | protected function getMutexPath() |
||
221 | |||
222 | /** |
||
223 | * Do not allow the event to overlap each other. |
||
224 | * |
||
225 | * @return $this |
||
226 | */ |
||
227 | public function preventOverlapping() |
||
238 | |||
239 | /** |
||
240 | * Tells you whether this event has been denied from mutual exclusiveness |
||
241 | * |
||
242 | * @return bool |
||
243 | */ |
||
244 | protected function isLocked() |
||
248 | |||
249 | /** |
||
250 | * Schedule the event to run between start and end time. |
||
251 | * |
||
252 | * @param string $startTime |
||
253 | * @param string $endTime |
||
254 | * @return $this |
||
255 | */ |
||
256 | public function between($startTime, $endTime) |
||
260 | |||
261 | /** |
||
262 | * Schedule the event to not run between start and end time. |
||
263 | * |
||
264 | * @param string $startTime |
||
265 | * @param string $endTime |
||
266 | * @return $this |
||
267 | */ |
||
268 | public function notBetween($startTime, $endTime) |
||
272 | |||
273 | /** |
||
274 | * Schedule the event to run between start and end time. |
||
275 | * |
||
276 | * @param string $startTime |
||
277 | * @param string $endTime |
||
278 | * @return \Closure |
||
279 | */ |
||
280 | private function inTimeInterval($startTime, $endTime) |
||
287 | |||
288 | /** |
||
289 | * State that the command should run in the foreground |
||
290 | * |
||
291 | * @return $this |
||
292 | */ |
||
293 | public function runInForeground() |
||
299 | |||
300 | /** |
||
301 | * State that the command should run in the background. |
||
302 | * |
||
303 | * @return $this |
||
304 | */ |
||
305 | public function runInBackground() |
||
311 | |||
312 | /** |
||
313 | * Set the timezone the date should be evaluated on. |
||
314 | * |
||
315 | * @return $this |
||
316 | */ |
||
317 | public function timezone(\DateTimeZone $timezone) |
||
323 | |||
324 | /** |
||
325 | * Set which user the command should run as. |
||
326 | * |
||
327 | * @param string? $user |
||
328 | * @return $this |
||
329 | */ |
||
330 | public function asUser(/* string? */ $user) |
||
338 | |||
339 | /** |
||
340 | * Determine if the given event should run based on the Cron expression. |
||
341 | * |
||
342 | * @return bool |
||
343 | */ |
||
344 | public function isDue() |
||
348 | |||
349 | /** |
||
350 | * Determine if the Cron expression passes. |
||
351 | * |
||
352 | * @return boolean |
||
353 | */ |
||
354 | protected function expressionPasses() |
||
364 | |||
365 | /** |
||
366 | * Determine if the filters pass for the event. |
||
367 | * |
||
368 | * @return boolean |
||
369 | */ |
||
370 | protected function filtersPass() |
||
386 | |||
387 | /** |
||
388 | * Change the current working directory. |
||
389 | * |
||
390 | * @param string $directory |
||
391 | * @return $this |
||
392 | */ |
||
393 | public function in(/* string */ $directory) |
||
401 | |||
402 | /** |
||
403 | * Whether we append or redirect output |
||
404 | * |
||
405 | * @param bool $switch |
||
406 | * @return $this |
||
407 | */ |
||
408 | public function appendOutput(/* boolean */ $switch = true) |
||
416 | |||
417 | /** |
||
418 | * Set the file or location where to send file descriptor 1 to |
||
419 | * |
||
420 | * @param string $output |
||
421 | * @return $this |
||
422 | */ |
||
423 | public function outputTo(/* string */ $output) |
||
431 | |||
432 | /** |
||
433 | * Set the file or location where to send file descriptor 2 to |
||
434 | * |
||
435 | * @param string $output |
||
436 | * @return $this |
||
437 | */ |
||
438 | public function errorOutputTo(/* string */ $output) |
||
446 | |||
447 | /** |
||
448 | * The Cron expression representing the event's frequency. |
||
449 | * |
||
450 | * @param string $expression |
||
451 | * @return $this |
||
452 | */ |
||
453 | public function cron(/* string */ $expression) |
||
461 | |||
462 | /** |
||
463 | * @return CronExpression |
||
464 | */ |
||
465 | public function getCronExpression() |
||
469 | |||
470 | /** |
||
471 | * Change the minute when the job should run (0-59, *, *\/2 etc) |
||
472 | * |
||
473 | * @param string|int $minute |
||
474 | * @return $this |
||
475 | */ |
||
476 | public function minute($minute) |
||
480 | |||
481 | /** |
||
482 | * Schedule the event to run every minute. |
||
483 | * |
||
484 | * @return $this |
||
485 | */ |
||
486 | public function everyMinute() |
||
490 | |||
491 | /** |
||
492 | * Schedule this event to run every 5 minutes |
||
493 | * |
||
494 | * @return Event |
||
495 | */ |
||
496 | public function everyFiveMinutes() |
||
500 | |||
501 | /** |
||
502 | * Schedule the event to run every N minutes |
||
503 | * |
||
504 | * @param int $n |
||
505 | * @return $this |
||
506 | */ |
||
507 | public function everyNMinutes(/* int */ $n) |
||
513 | |||
514 | /** |
||
515 | * Set the hour when the job should run (0-23, *, *\/2, etc) |
||
516 | * |
||
517 | * @param string|int $hour |
||
518 | * @return Event |
||
519 | */ |
||
520 | public function hour($hour) |
||
524 | |||
525 | /** |
||
526 | * Schedule the event to run hourly. |
||
527 | * |
||
528 | * @return $this |
||
529 | */ |
||
530 | public function hourly() |
||
536 | |||
537 | /** |
||
538 | * Schedule the event to run daily. |
||
539 | * |
||
540 | * @return $this |
||
541 | */ |
||
542 | public function daily() |
||
548 | |||
549 | /** |
||
550 | * Schedule the event to run daily at a given time (10:00, 19:30, etc). |
||
551 | * |
||
552 | * @param string $time |
||
553 | * @return $this |
||
554 | */ |
||
555 | public function dailyAt(/* string */ $time) |
||
564 | |||
565 | /** |
||
566 | * Set the days of the week the command should run on. |
||
567 | * |
||
568 | * @param array|mixed $days |
||
569 | * @return $this |
||
570 | */ |
||
571 | public function days($days) |
||
577 | |||
578 | /** |
||
579 | * Schedule the event to run only on weekdays. |
||
580 | * |
||
581 | * @return $this |
||
582 | */ |
||
583 | public function weekdays() |
||
587 | |||
588 | /** |
||
589 | * Schedule the event to run only on Mondays. |
||
590 | * |
||
591 | * @return $this |
||
592 | */ |
||
593 | public function mondays() |
||
597 | |||
598 | /** |
||
599 | * Schedule the event to run only on Tuesdays. |
||
600 | * |
||
601 | * @return $this |
||
602 | */ |
||
603 | public function tuesdays() |
||
607 | |||
608 | /** |
||
609 | * Schedule the event to run only on Wednesdays. |
||
610 | * |
||
611 | * @return $this |
||
612 | */ |
||
613 | public function wednesdays() |
||
617 | |||
618 | /** |
||
619 | * Schedule the event to run only on Thursdays. |
||
620 | * |
||
621 | * @return $this |
||
622 | */ |
||
623 | public function thursdays() |
||
627 | |||
628 | /** |
||
629 | * Schedule the event to run only on Fridays. |
||
630 | * |
||
631 | * @return $this |
||
632 | */ |
||
633 | public function fridays() |
||
637 | |||
638 | /** |
||
639 | * Schedule the event to run only on Saturdays. |
||
640 | * |
||
641 | * @return $this |
||
642 | */ |
||
643 | public function saturdays() |
||
647 | |||
648 | /** |
||
649 | * Schedule the event to run only on Sundays. |
||
650 | * |
||
651 | * @return $this |
||
652 | */ |
||
653 | public function sundays() |
||
657 | |||
658 | /** |
||
659 | * Schedule the event to run weekly. |
||
660 | * |
||
661 | * @return $this |
||
662 | */ |
||
663 | public function weekly() |
||
669 | |||
670 | /** |
||
671 | * Schedule the event to run weekly on a given day and time. |
||
672 | * |
||
673 | * @param int $day |
||
674 | * @param string $time |
||
675 | * @return $this |
||
676 | */ |
||
677 | public function weeklyOn($day, $time = '0:0') |
||
682 | |||
683 | /** |
||
684 | * Schedule the event to run monthly. |
||
685 | * |
||
686 | * @return $this |
||
687 | */ |
||
688 | public function monthly() |
||
694 | |||
695 | /** |
||
696 | * Schedule the event to run monthly on a given day and time. |
||
697 | * |
||
698 | * @param int $day |
||
699 | * @param string $time |
||
700 | * @return $this |
||
701 | */ |
||
702 | public function monthlyOn($day = 1, $time = '0:0') |
||
707 | |||
708 | /** |
||
709 | * Schedule the event to run quarterly. |
||
710 | * |
||
711 | * @return $this |
||
712 | */ |
||
713 | public function quarterly() |
||
720 | |||
721 | /** |
||
722 | * Schedule the event to run yearly. |
||
723 | * |
||
724 | * @return $this |
||
725 | */ |
||
726 | public function yearly() |
||
733 | |||
734 | /** |
||
735 | * Splice the given value into the given position of the expression. |
||
736 | * |
||
737 | * @param int $position |
||
738 | * @param string $value |
||
739 | * @return $this |
||
740 | */ |
||
741 | protected function spliceIntoPosition($position, $value) |
||
747 | |||
748 | /** |
||
749 | * Register a callback to further filter the schedule. |
||
750 | * |
||
751 | * @param \Closure $callback |
||
752 | * @return $this |
||
753 | */ |
||
754 | public function when(Closure $callback) |
||
760 | |||
761 | /** |
||
762 | * Register a callback to further filter the schedule. |
||
763 | * |
||
764 | * @param \Closure $callback |
||
765 | * @return $this |
||
766 | */ |
||
767 | public function skip(Closure $callback) |
||
773 | |||
774 | /** |
||
775 | * Register a callback to be called before the operation. |
||
776 | * |
||
777 | * @param \Closure $callback |
||
778 | * @return $this |
||
779 | */ |
||
780 | public function before(Closure $callback) |
||
786 | |||
787 | /** |
||
788 | * Register a callback to be called after the operation. |
||
789 | * |
||
790 | * @param \Closure $callback |
||
791 | * @return $this |
||
792 | */ |
||
793 | public function after(Closure $callback) |
||
799 | } |
||
800 |
Overly long lines are hard to read on any screen. Most code styles therefor impose a maximum limit on the number of characters in a line.