Complex classes like LogCommand 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 LogCommand, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
27 | class LogCommand extends AbstractCommand implements IAggregatorAwareCommand, IConfigAwareCommand |
||
28 | { |
||
29 | |||
30 | const SETTING_LOG_LIMIT = 'log.limit'; |
||
31 | |||
32 | const SETTING_LOG_MESSAGE_LIMIT = 'log.message-limit'; |
||
33 | |||
34 | const SETTING_LOG_MERGE_CONFLICT_REGEXPS = 'log.merge-conflict-regexps'; |
||
35 | |||
36 | const ALL_REFS = 'all'; |
||
37 | |||
38 | /** |
||
39 | * Revision list parser. |
||
40 | * |
||
41 | * @var RevisionListParser |
||
42 | */ |
||
43 | private $_revisionListParser; |
||
44 | |||
45 | /** |
||
46 | * Revision log |
||
47 | * |
||
48 | * @var RevisionLog |
||
49 | */ |
||
50 | private $_revisionLog; |
||
51 | |||
52 | /** |
||
53 | * Revision printer. |
||
54 | * |
||
55 | * @var RevisionPrinter |
||
56 | */ |
||
57 | private $_revisionPrinter; |
||
58 | |||
59 | /** |
||
60 | * Prepare dependencies. |
||
61 | * |
||
62 | * @return void |
||
63 | */ |
||
64 | protected function prepareDependencies() |
||
65 | { |
||
66 | parent::prepareDependencies(); |
||
67 | |||
68 | $container = $this->getContainer(); |
||
69 | |||
70 | $this->_revisionListParser = $container['revision_list_parser']; |
||
71 | $this->_revisionPrinter = $container['revision_printer']; |
||
72 | } |
||
73 | |||
74 | /** |
||
75 | * {@inheritdoc} |
||
76 | */ |
||
77 | protected function configure() |
||
78 | { |
||
79 | $this->pathAcceptsUrl = true; |
||
80 | |||
81 | $this |
||
82 | ->setName('log') |
||
83 | ->setDescription( |
||
84 | 'Show the log messages for a set of revisions, bugs, paths, refs, etc.' |
||
85 | ) |
||
86 | ->addArgument( |
||
87 | 'path', |
||
88 | InputArgument::OPTIONAL, |
||
89 | 'Working copy path or URL', |
||
90 | '.' |
||
91 | ) |
||
92 | ->addOption( |
||
93 | 'revisions', |
||
94 | 'r', |
||
95 | InputOption::VALUE_REQUIRED, |
||
96 | 'List of revision(-s) and/or revision range(-s), e.g. <comment>53324</comment>, <comment>1224-4433</comment>' |
||
97 | ) |
||
98 | ->addOption( |
||
99 | 'bugs', |
||
100 | 'b', |
||
101 | InputOption::VALUE_REQUIRED, |
||
102 | 'List of bug(-s), e.g. <comment>JRA-1234</comment>, <comment>43644</comment>' |
||
103 | ) |
||
104 | ->addOption( |
||
105 | 'refs', |
||
106 | null, |
||
107 | InputOption::VALUE_REQUIRED, |
||
108 | 'List of refs, e.g. <comment>trunk</comment>, <comment>branches/branch-name</comment>, <comment>tags/tag-name</comment> or <comment>all</comment> for all refs' |
||
109 | ) |
||
110 | ->addOption( |
||
111 | 'merges', |
||
112 | null, |
||
113 | InputOption::VALUE_NONE, |
||
114 | 'Show merge revisions only' |
||
115 | ) |
||
116 | ->addOption( |
||
117 | 'no-merges', |
||
118 | null, |
||
119 | InputOption::VALUE_NONE, |
||
120 | 'Hide merge revisions' |
||
121 | ) |
||
122 | ->addOption( |
||
123 | 'merged', |
||
124 | null, |
||
125 | InputOption::VALUE_NONE, |
||
126 | 'Shows only revisions, that were merged at least once' |
||
127 | ) |
||
128 | ->addOption( |
||
129 | 'not-merged', |
||
130 | null, |
||
131 | InputOption::VALUE_NONE, |
||
132 | 'Shows only revisions, that were not merged' |
||
133 | ) |
||
134 | ->addOption( |
||
135 | 'merged-by', |
||
136 | null, |
||
137 | InputOption::VALUE_REQUIRED, |
||
138 | 'Show revisions merged by list of revision(-s) and/or revision range(-s)' |
||
139 | ) |
||
140 | ->addOption( |
||
141 | 'action', |
||
142 | null, |
||
143 | InputOption::VALUE_REQUIRED, |
||
144 | 'Show revisions, whose paths were affected by specified action, e.g. <comment>A</comment>, <comment>M</comment>, <comment>R</comment>, <comment>D</comment>' |
||
145 | ) |
||
146 | ->addOption( |
||
147 | 'kind', |
||
148 | null, |
||
149 | InputOption::VALUE_REQUIRED, |
||
150 | 'Show revisions, whose paths match specified kind, e.g. <comment>dir</comment> or <comment>file</comment>' |
||
151 | ) |
||
152 | ->addOption( |
||
153 | 'with-details', |
||
154 | 'd', |
||
155 | InputOption::VALUE_NONE, |
||
156 | 'Shows detailed revision information, e.g. paths affected' |
||
157 | ) |
||
158 | ->addOption( |
||
159 | 'with-summary', |
||
160 | 's', |
||
161 | InputOption::VALUE_NONE, |
||
162 | 'Shows number of added/changed/removed paths in the revision' |
||
163 | ) |
||
164 | ->addOption( |
||
165 | 'with-refs', |
||
166 | null, |
||
167 | InputOption::VALUE_NONE, |
||
168 | 'Shows revision refs' |
||
169 | ) |
||
170 | ->addOption( |
||
171 | 'with-merge-oracle', |
||
172 | null, |
||
173 | InputOption::VALUE_NONE, |
||
174 | 'Shows number of paths in the revision, that can cause conflict upon merging' |
||
175 | ) |
||
176 | ->addOption( |
||
177 | 'with-merge-status', |
||
178 | null, |
||
179 | InputOption::VALUE_NONE, |
||
180 | 'Shows merge revisions affecting this revision' |
||
181 | ) |
||
182 | ->addOption( |
||
183 | 'max-count', |
||
184 | null, |
||
185 | InputOption::VALUE_REQUIRED, |
||
186 | 'Limit the number of revisions to output' |
||
187 | ); |
||
188 | |||
189 | parent::configure(); |
||
190 | } |
||
191 | |||
192 | /** |
||
193 | * Return possible values for the named option |
||
194 | * |
||
195 | * @param string $optionName Option name. |
||
196 | * @param CompletionContext $context Completion context. |
||
197 | * |
||
198 | * @return array |
||
199 | */ |
||
200 | public function completeOptionValues($optionName, CompletionContext $context) |
||
201 | { |
||
202 | $ret = parent::completeOptionValues($optionName, $context); |
||
203 | |||
204 | if ( $optionName === 'refs' ) { |
||
205 | return $this->getAllRefs(); |
||
206 | } |
||
207 | elseif ( $optionName === 'action' ) { |
||
208 | return $this->getAllActions(); |
||
209 | } |
||
210 | elseif ( $optionName === 'kind' ) { |
||
211 | return $this->getAllKinds(); |
||
212 | } |
||
213 | |||
214 | return $ret; |
||
215 | } |
||
216 | |||
217 | /** |
||
218 | * {@inheritdoc} |
||
219 | */ |
||
220 | public function initialize(InputInterface $input, OutputInterface $output) |
||
226 | |||
227 | /** |
||
228 | * {@inheritdoc} |
||
229 | * |
||
230 | * @throws \RuntimeException When both "--bugs" and "--revisions" options were specified. |
||
231 | * @throws CommandException When specified revisions are not present in current project. |
||
232 | * @throws CommandException When project contains no associated revisions. |
||
233 | */ |
||
234 | protected function execute(InputInterface $input, OutputInterface $output) |
||
358 | |||
359 | /** |
||
360 | * Returns all refs. |
||
361 | * |
||
362 | * @return array |
||
363 | */ |
||
364 | protected function getAllRefs() |
||
371 | |||
372 | /** |
||
373 | * Returns all actions. |
||
374 | * |
||
375 | * @return array |
||
376 | */ |
||
377 | protected function getAllActions() |
||
381 | |||
382 | /** |
||
383 | * Returns all actions. |
||
384 | * |
||
385 | * @return array |
||
386 | */ |
||
387 | protected function getAllKinds() |
||
391 | |||
392 | /** |
||
393 | * Returns revision log identifier. |
||
394 | * |
||
395 | * @return string |
||
396 | */ |
||
397 | protected function getRevisionLogIdentifier() |
||
412 | |||
413 | /** |
||
414 | * Shows error about missing revisions. |
||
415 | * |
||
416 | * @param array $missing_revisions Missing revisions. |
||
417 | * |
||
418 | * @return string |
||
419 | */ |
||
420 | protected function getMissingRevisionsErrorMessage(array $missing_revisions) |
||
434 | |||
435 | /** |
||
436 | * Returns list of revisions by path. |
||
437 | * |
||
438 | * @return array |
||
439 | * @throws CommandException When given path doesn't exist. |
||
440 | * @throws CommandException When given refs doesn't exist. |
||
441 | */ |
||
442 | protected function getRevisionsByPath() |
||
477 | |||
478 | /** |
||
479 | * Returns difference between 2 paths. |
||
480 | * |
||
481 | * @param string $main_path Main path. |
||
482 | * @param string $sub_path Sub path. |
||
483 | * |
||
484 | * @return string |
||
485 | */ |
||
486 | private function _getPathDifference($main_path, $sub_path) |
||
511 | |||
512 | /** |
||
513 | * Determines path kind from repository. |
||
514 | * |
||
515 | * @param string $path Path. |
||
516 | * |
||
517 | * @return string|null |
||
518 | */ |
||
519 | private function _crossReferencePathFromRepository($path) |
||
532 | |||
533 | /** |
||
534 | * Returns displayed revision limit. |
||
535 | * |
||
536 | * @return integer |
||
537 | */ |
||
538 | protected function getMaxCount() |
||
548 | |||
549 | /** |
||
550 | * Prints revisions. |
||
551 | * |
||
552 | * @param array $revisions Revisions. |
||
553 | * |
||
554 | * @return void |
||
555 | */ |
||
556 | protected function printRevisions(array $revisions) |
||
585 | |||
586 | /** |
||
587 | * Returns list of config settings. |
||
588 | * |
||
589 | * @return AbstractConfigSetting[] |
||
590 | */ |
||
591 | public function getConfigSettings() |
||
599 | |||
600 | /** |
||
601 | * Returns option names, that makes sense to use in aggregation mode. |
||
602 | * |
||
603 | * @return array |
||
604 | */ |
||
605 | public function getAggregatedOptions() |
||
613 | |||
614 | } |
||
615 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)
or! empty(...)
instead.