Complex classes like BugsPlugin 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 BugsPlugin, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
20 | class BugsPlugin extends AbstractDatabaseCollectorPlugin |
||
21 | { |
||
22 | |||
23 | const STATISTIC_BUG_ADDED_TO_COMMIT = 'bug_added_to_commit'; |
||
24 | |||
25 | /** |
||
26 | * Repository url. |
||
27 | * |
||
28 | * @var string |
||
29 | */ |
||
30 | private $_repositoryUrl; |
||
31 | |||
32 | /** |
||
33 | * Repository connector. |
||
34 | * |
||
35 | * @var Connector |
||
36 | */ |
||
37 | private $_repositoryConnector; |
||
38 | |||
39 | /** |
||
40 | * Log message parser factory. |
||
41 | * |
||
42 | * @var LogMessageParserFactory |
||
43 | */ |
||
44 | private $_logMessageParserFactory; |
||
45 | |||
46 | /** |
||
47 | * Create bugs revision log plugin. |
||
48 | * |
||
49 | * @param ExtendedPdoInterface $database Database. |
||
50 | * @param RepositoryFiller $repository_filler Repository filler. |
||
51 | * @param string $repository_url Repository url. |
||
52 | * @param Connector $repository_connector Repository connector. |
||
53 | * @param LogMessageParserFactory $log_message_parser_factory Log message parser. |
||
54 | */ |
||
55 | 19 | public function __construct( |
|
68 | |||
69 | /** |
||
70 | * Returns plugin name. |
||
71 | * |
||
72 | * @return string |
||
73 | */ |
||
74 | 13 | public function getName() |
|
78 | |||
79 | /** |
||
80 | * Defines parsing statistic types. |
||
81 | * |
||
82 | * @return array |
||
83 | */ |
||
84 | 19 | public function defineStatisticTypes() |
|
85 | { |
||
86 | return array( |
||
87 | 19 | self::STATISTIC_BUG_ADDED_TO_COMMIT, |
|
88 | ); |
||
89 | } |
||
90 | |||
91 | /** |
||
92 | * Processes data. |
||
93 | * |
||
94 | * @param integer $from_revision From revision. |
||
95 | * @param integer $to_revision To revision. |
||
96 | * |
||
97 | * @return void |
||
98 | */ |
||
99 | 9 | public function doProcess($from_revision, $to_revision) |
|
100 | { |
||
101 | 9 | $this->populateMissingBugRegExp(); |
|
102 | |||
103 | 9 | $last_revision = $this->getLastRevision(); |
|
104 | |||
105 | 9 | if ( $to_revision > $last_revision ) { |
|
106 | 7 | $this->detectBugs($last_revision + 1, $to_revision); |
|
107 | 7 | $this->setLastRevision($to_revision); |
|
108 | } |
||
109 | 9 | } |
|
110 | |||
111 | /** |
||
112 | * Populate "BugRegExp" column for projects without it. |
||
113 | * |
||
114 | * @return void |
||
115 | */ |
||
116 | 9 | protected function populateMissingBugRegExp() |
|
117 | { |
||
118 | 9 | $projects = $this->getProjects('BugRegExp IS NULL'); |
|
119 | |||
120 | 9 | if ( !$projects ) { |
|
121 | 5 | $this->advanceProgressBar(); |
|
122 | |||
123 | 5 | return; |
|
124 | } |
||
125 | |||
126 | 4 | foreach ( $projects as $project_data ) { |
|
127 | 4 | $bug_regexp = $this->detectProjectBugTraqRegEx( |
|
128 | 4 | $project_data['Path'], |
|
129 | 4 | $project_data['RevisionLastSeen'], |
|
130 | 4 | (bool)$project_data['IsDeleted'] |
|
131 | ); |
||
132 | |||
133 | 4 | $this->repositoryFiller->setProjectBugRegexp($project_data['Id'], $bug_regexp); |
|
|
|||
134 | 4 | $this->advanceProgressBar(); |
|
135 | } |
||
136 | 4 | } |
|
137 | |||
138 | /** |
||
139 | * Determines project bug tracking regular expression. |
||
140 | * |
||
141 | * @param string $project_path Project project_path. |
||
142 | * @param integer $revision Revision. |
||
143 | * @param boolean $project_deleted Project is deleted. |
||
144 | * |
||
145 | * @return string |
||
146 | */ |
||
147 | 4 | protected function detectProjectBugTraqRegEx($project_path, $revision, $project_deleted) |
|
148 | { |
||
149 | 4 | $ref_paths = $this->getLastChangedRefPaths($project_path); |
|
150 | |||
151 | 4 | if ( !$ref_paths ) { |
|
152 | 2 | return ''; |
|
153 | } |
||
154 | |||
155 | 2 | foreach ( $ref_paths as $ref_path ) { |
|
156 | 2 | $logregex = $this->_repositoryConnector |
|
157 | 2 | ->withCache('1 year') |
|
158 | 2 | ->getProperty( |
|
159 | 2 | 'bugtraq:logregex', |
|
160 | 2 | $this->_repositoryUrl . $ref_path . ($project_deleted ? '@' . $revision : '') |
|
161 | ); |
||
162 | |||
163 | 2 | if ( strlen($logregex) ) { |
|
164 | 2 | return $logregex; |
|
165 | } |
||
166 | } |
||
167 | |||
168 | return ''; |
||
169 | } |
||
170 | |||
171 | /** |
||
172 | * Returns given project refs, where last changed are on top. |
||
173 | * |
||
174 | * @param string $project_path Path. |
||
175 | * |
||
176 | * @return array |
||
177 | */ |
||
178 | 4 | protected function getLastChangedRefPaths($project_path) |
|
179 | { |
||
180 | 4 | $own_nesting_level = substr_count($project_path, '/') - 1; |
|
181 | |||
182 | $where_clause = array( |
||
183 | 4 | 'Path LIKE :parent_path', |
|
184 | 'PathNestingLevel BETWEEN :from_level AND :to_level', |
||
185 | 'RevisionDeleted IS NULL', |
||
186 | ); |
||
187 | |||
188 | $sql = 'SELECT Path, RevisionLastSeen |
||
189 | FROM Paths |
||
190 | 4 | WHERE (' . implode(') AND (', $where_clause) . ')'; |
|
191 | 4 | $paths = $this->database->fetchPairs($sql, array( |
|
192 | 4 | 'parent_path' => $project_path . '%', |
|
193 | 4 | 'from_level' => $own_nesting_level + 1, |
|
194 | 4 | 'to_level' => $own_nesting_level + 2, |
|
195 | )); |
||
196 | |||
197 | // No sub-folders. |
||
198 | 4 | if ( !$paths ) { |
|
199 | 1 | return array(); |
|
200 | } |
||
201 | |||
202 | 3 | $filtered_paths = array(); |
|
203 | |||
204 | 3 | foreach ( $paths as $path => $revision ) { |
|
205 | 3 | if ( $this->isRef($path) ) { |
|
206 | 3 | $filtered_paths[$path] = $revision; |
|
207 | } |
||
208 | } |
||
209 | |||
210 | // None of sub-folders matches a ref. |
||
211 | 3 | if ( !$filtered_paths ) { |
|
212 | 1 | return array(); |
|
213 | } |
||
214 | |||
215 | 2 | arsort($filtered_paths, SORT_NUMERIC); |
|
216 | |||
217 | 2 | return array_keys($filtered_paths); |
|
218 | } |
||
219 | |||
220 | /** |
||
221 | * Detects if given project_path is known project root. |
||
222 | * |
||
223 | * @param string $path Path. |
||
224 | * |
||
225 | * @return boolean |
||
226 | */ |
||
227 | 3 | protected function isRef($path) |
|
236 | |||
237 | /** |
||
238 | * Detects bugs, associated with each commit from a given revision range. |
||
239 | * |
||
240 | * @param integer $from_revision From revision. |
||
241 | * @param integer $to_revision To revision. |
||
242 | * |
||
243 | * @return void |
||
244 | */ |
||
245 | 7 | protected function detectBugs($from_revision, $to_revision) |
|
246 | { |
||
247 | 7 | $bug_regexp_mapping = $this->getProjectBugRegExps(); |
|
248 | |||
249 | 7 | if ( !$bug_regexp_mapping ) { |
|
250 | 3 | $this->advanceProgressBar(); |
|
251 | |||
252 | 3 | return; |
|
253 | } |
||
254 | |||
255 | 4 | $range_start = $from_revision; |
|
256 | |||
257 | 4 | while ( $range_start <= $to_revision ) { |
|
258 | 4 | $range_end = min($range_start + 999, $to_revision); |
|
259 | |||
260 | 4 | $this->doDetectBugs($range_start, $range_end, $bug_regexp_mapping); |
|
261 | 4 | $this->advanceProgressBar(); |
|
262 | |||
263 | 4 | $range_start = $range_end + 1; |
|
264 | } |
||
265 | 4 | } |
|
266 | |||
267 | /** |
||
268 | * Returns "BugRegExp" field associated with every project. |
||
269 | * |
||
270 | * @return array |
||
271 | */ |
||
272 | 7 | protected function getProjectBugRegExps() |
|
273 | { |
||
274 | 7 | $projects = $this->getProjects("BugRegExp != ''"); |
|
275 | |||
276 | 7 | if ( !$projects ) { |
|
277 | 3 | return array(); |
|
278 | } |
||
279 | |||
280 | 4 | $ret = array(); |
|
281 | |||
282 | 4 | foreach ( $projects as $project_data ) { |
|
283 | 4 | $ret[$project_data['Id']] = $project_data['BugRegExp']; |
|
284 | } |
||
285 | |||
286 | 4 | return $ret; |
|
287 | } |
||
288 | |||
289 | /** |
||
290 | * Detects bugs, associated with each commit from a given revision range. |
||
291 | * |
||
292 | * @param integer $from_revision From revision. |
||
293 | * @param integer $to_revision To revision. |
||
294 | * @param array $bug_regexp_mapping Mapping between project and it's "BugRegExp" field. |
||
295 | * |
||
296 | * @return void |
||
297 | */ |
||
298 | 4 | protected function doDetectBugs($from_revision, $to_revision, array $bug_regexp_mapping) |
|
299 | { |
||
300 | 4 | $commits_by_project = $this->getCommitsGroupedByProject($from_revision, $to_revision); |
|
301 | |||
302 | 4 | foreach ( $commits_by_project as $project_id => $project_commits ) { |
|
303 | 4 | if ( !isset($bug_regexp_mapping[$project_id]) ) { |
|
304 | 1 | continue; |
|
305 | } |
||
306 | |||
307 | 4 | $log_message_parser = $this->_logMessageParserFactory->getLogMessageParser( |
|
308 | 4 | $bug_regexp_mapping[$project_id] |
|
309 | ); |
||
310 | |||
311 | 4 | foreach ( $project_commits as $revision => $log_message ) { |
|
312 | 4 | $bugs = $log_message_parser->parse($log_message); |
|
313 | |||
314 | 4 | if ( $bugs ) { |
|
315 | 3 | $this->repositoryFiller->addBugsToCommit($bugs, $revision); |
|
316 | 4 | $this->recordStatistic(self::STATISTIC_BUG_ADDED_TO_COMMIT, count($bugs)); |
|
317 | } |
||
318 | } |
||
319 | } |
||
320 | 4 | } |
|
321 | |||
322 | /** |
||
323 | * Returns commits grouped by project. |
||
324 | * |
||
325 | * @param integer $from_revision From revision. |
||
326 | * @param integer $to_revision To revision. |
||
327 | * |
||
328 | * @return array |
||
329 | */ |
||
330 | 4 | protected function getCommitsGroupedByProject($from_revision, $to_revision) |
|
331 | { |
||
332 | 4 | $sql = 'SELECT cp.Revision, c.Message, cp.ProjectId |
|
333 | FROM CommitProjects cp |
||
334 | JOIN Commits c ON c.Revision = cp.Revision |
||
335 | WHERE cp.Revision BETWEEN :from_revision AND :to_revision'; |
||
336 | 4 | $commits = new AllIterator($this->database->perform($sql, array( |
|
337 | 4 | 'from_revision' => $from_revision, |
|
338 | 4 | 'to_revision' => $to_revision, |
|
339 | ))); |
||
340 | |||
341 | 4 | $ret = array(); |
|
342 | 4 | $processed_revisions = array(); |
|
343 | |||
344 | 4 | foreach ( $commits as $commit_data ) { |
|
345 | 4 | $revision = $commit_data['Revision']; |
|
346 | |||
347 | // Don't process revision more then once (e.g. when commit belongs to different projects). |
||
348 | 4 | if ( isset($processed_revisions[$revision]) ) { |
|
349 | 1 | continue; |
|
350 | } |
||
351 | |||
352 | 4 | $project_id = $commit_data['ProjectId']; |
|
353 | |||
354 | 4 | if ( !isset($ret[$project_id]) ) { |
|
355 | 4 | $ret[$project_id] = array(); |
|
356 | } |
||
357 | |||
358 | 4 | $ret[$project_id][$revision] = $commit_data['Message']; |
|
359 | 4 | $processed_revisions[$revision] = true; |
|
360 | } |
||
361 | |||
362 | 4 | return $ret; |
|
363 | } |
||
364 | |||
365 | /** |
||
366 | * Find revisions by collected data. |
||
367 | * |
||
368 | * @param array $criteria Criteria. |
||
369 | * @param string $project_path Project path. |
||
370 | * |
||
371 | * @return array |
||
372 | */ |
||
373 | 5 | public function find(array $criteria, $project_path) |
|
391 | |||
392 | /** |
||
393 | * Returns information about revisions. |
||
394 | * |
||
395 | * @param array $revisions Revisions. |
||
396 | * |
||
397 | * @return array |
||
398 | */ |
||
399 | 1 | public function getRevisionsData(array $revisions) |
|
400 | { |
||
421 | |||
422 | } |
||
423 |
If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:
If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.