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 Search 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 Search, and based on these observations, apply Extract Interface, too.
1 | <?php namespace Myth\Docs; |
||
43 | class Search implements DocSearchInterface |
||
44 | { |
||
45 | |||
46 | /** |
||
47 | * Minimum characters that can be submitted for a search. |
||
48 | * |
||
49 | * @var int |
||
50 | */ |
||
51 | protected $min_chars = 3; |
||
52 | |||
53 | /** |
||
54 | * Maximum characters that can be submitted for a search. |
||
55 | * |
||
56 | * @var int |
||
57 | */ |
||
58 | protected $max_chars = 30; |
||
59 | |||
60 | /** |
||
61 | * Valid file extensions we can search in. |
||
62 | * |
||
63 | * @var string |
||
64 | */ |
||
65 | protected $allowed_file_types = 'html|htm|php|php4|php5|txt|md'; |
||
66 | |||
67 | /** |
||
68 | * Which files should we skip over during our search? |
||
69 | * |
||
70 | * @var array |
||
71 | */ |
||
72 | protected $skip_files = ['.', '..', '_404.md', '_toc.ini']; |
||
73 | |||
74 | /** |
||
75 | * How much of each file should we read. |
||
76 | * Use lower values for faster searches. |
||
77 | * |
||
78 | * @var int |
||
79 | */ |
||
80 | protected $byte_size = 51200; |
||
81 | |||
82 | /** |
||
83 | * Number of words long (approximately) |
||
84 | * the result excerpt should be. |
||
85 | * |
||
86 | * @var int |
||
87 | */ |
||
88 | protected $excerpt_length = 60; |
||
89 | |||
90 | /** |
||
91 | * The maximum number of results allowed from a single file. |
||
92 | * |
||
93 | * @var int |
||
94 | */ |
||
95 | protected $max_per_file = 1; |
||
96 | |||
97 | protected $doc_folders = array(); |
||
98 | |||
99 | protected $formatters = array(); |
||
100 | |||
101 | //-------------------------------------------------------------------- |
||
102 | |||
103 | /** |
||
104 | * The entry point for performing a search of the documentation. |
||
105 | * |
||
106 | * @param null $terms |
||
107 | * @param array $folders |
||
108 | * |
||
109 | * @return array|null |
||
110 | */ |
||
111 | public function search($terms = null, $folders = []) |
||
126 | |||
127 | //-------------------------------------------------------------------- |
||
128 | |||
129 | //-------------------------------------------------------------------- |
||
130 | // Private Methods |
||
131 | //-------------------------------------------------------------------- |
||
132 | |||
133 | |||
134 | /** |
||
135 | * Searches a single directory worth of files. |
||
136 | * |
||
137 | * @param $term |
||
138 | * @param $folder |
||
139 | * @param $group_name |
||
140 | * |
||
141 | * @return array The results. |
||
142 | */ |
||
143 | protected function searchFolder($term, $folder, $group_name) |
||
227 | |||
228 | //-------------------------------------------------------------------- |
||
229 | |||
230 | /** |
||
231 | * Stores the name of the callback method to run to convert the source |
||
232 | * files to viewable files. By default, this should be used to register |
||
233 | * a Mardown Extended formatter with the system, but could be used to |
||
234 | * extend the |
||
235 | * |
||
236 | * @param string $callback_name |
||
237 | * @param bool $cascade // If FALSE the formatting of a component ends here. If TRUE, will be passed to next formatter. |
||
238 | * @return $this |
||
239 | */ |
||
240 | public function registerFormatter($callback_name = '', $cascade = false) |
||
248 | |||
249 | //-------------------------------------------------------------------- |
||
250 | |||
251 | /** |
||
252 | * Runs the text through the registered formatters. |
||
253 | * |
||
254 | * @param $str |
||
255 | * @return mixed |
||
256 | */ |
||
257 | View Code Duplication | public function format($str) |
|
272 | |||
273 | //-------------------------------------------------------------------- |
||
274 | |||
275 | |||
276 | //-------------------------------------------------------------------- |
||
277 | // Protected Methods |
||
278 | //-------------------------------------------------------------------- |
||
279 | |||
280 | /** |
||
281 | * Converts an array generated by directory_map into a flat array of |
||
282 | * folders, removing any nested folders and adding them to the path. |
||
283 | * |
||
284 | * @param $map |
||
285 | * @param $prefix Used to recursively add the folder name... |
||
286 | * @return mixed |
||
287 | */ |
||
288 | protected function flattenMap($map, $prefix = '') |
||
311 | |||
312 | //-------------------------------------------------------------------- |
||
313 | |||
314 | /** |
||
315 | * Handles extracting the text surrounding our match and basic match formatting. |
||
316 | * |
||
317 | * @param $excerpt |
||
318 | * @param $term |
||
319 | * @param $match_string |
||
320 | * |
||
321 | * @return string |
||
322 | */ |
||
323 | protected function buildExtract($excerpt, $term, $match_string) |
||
349 | |||
350 | //-------------------------------------------------------------------- |
||
351 | |||
352 | /** |
||
353 | * Extracts the title from a bit of markdown formatted text. If it doesn't |
||
354 | * have an h1 or h2, then it uses the filename. |
||
355 | * |
||
356 | * @param $excerpt |
||
357 | * @param $file |
||
358 | * @return string |
||
359 | */ |
||
360 | protected function extractTitle($excerpt, $file) |
||
385 | //-------------------------------------------------------------------- |
||
386 | |||
387 | /** |
||
388 | * Create a Directory Map |
||
389 | * |
||
390 | * Reads the specified directory and builds an array |
||
391 | * representation of it. Sub-folders contained with the |
||
392 | * directory will be mapped as well. |
||
393 | * |
||
394 | * @param string $source_dir Path to source |
||
395 | * @param int $directory_depth Depth of directories to traverse |
||
396 | * (0 = fully recursive, 1 = current dir, etc) |
||
397 | * @param bool $hidden Whether to show hidden files |
||
398 | * @return array |
||
399 | */ |
||
400 | View Code Duplication | protected function directory_map($source_dir, $directory_depth = 0, $hidden = FALSE) |
|
433 | |||
434 | //-------------------------------------------------------------------- |
||
435 | |||
436 | /** |
||
437 | * Gets the first 'X' words of a string. |
||
438 | * |
||
439 | * @param $str |
||
440 | * @param int $wordCount |
||
441 | * @return string |
||
442 | */ |
||
443 | protected function firstXWords($str, $wordCount = 10) |
||
459 | |||
460 | //-------------------------------------------------------------------- |
||
461 | |||
462 | } |
||
463 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.