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 CHM 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 CHM, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class CHM |
||
15 | { |
||
16 | /** |
||
17 | * The reader that provides the data. |
||
18 | * |
||
19 | * @var Reader |
||
20 | */ |
||
21 | protected $reader; |
||
22 | |||
23 | /** |
||
24 | * The CHM initial header. |
||
25 | * |
||
26 | * @var Header\ITSF |
||
27 | */ |
||
28 | protected $itsf; |
||
29 | |||
30 | /** |
||
31 | * The directory listing header. |
||
32 | * |
||
33 | * @var Header\ITSP |
||
34 | */ |
||
35 | protected $itsp; |
||
36 | |||
37 | /** |
||
38 | * The entries found in this CHM. |
||
39 | * |
||
40 | * @var Entry[] |
||
41 | */ |
||
42 | protected $entries; |
||
43 | |||
44 | /** |
||
45 | * The data sections. |
||
46 | * |
||
47 | * @var Section\Section[] |
||
48 | */ |
||
49 | protected $sections; |
||
50 | |||
51 | /** |
||
52 | * The TOC. |
||
53 | * |
||
54 | * @var TOCIndex\Tree|null|false |
||
55 | */ |
||
56 | protected $toc; |
||
57 | |||
58 | /** |
||
59 | * The index. |
||
60 | * |
||
61 | * @var TOCIndex\Tree|null|false |
||
62 | */ |
||
63 | protected $index; |
||
64 | |||
65 | /** |
||
66 | * Initializes the instance. |
||
67 | * |
||
68 | * @param Reader $reader The reader that provides the data. |
||
69 | * |
||
70 | * @throws Exception Throws an Exception in case of errors. |
||
71 | */ |
||
72 | 1 | public function __construct(Reader $reader) |
|
105 | |||
106 | /** |
||
107 | * Destruct the instance. |
||
108 | */ |
||
109 | public function __destruct() |
||
113 | |||
114 | /** |
||
115 | * Create a new CHM instance reading a file. |
||
116 | * |
||
117 | * @param string $filename |
||
118 | * |
||
119 | * @return static |
||
120 | */ |
||
121 | 1 | public static function fromFile($filename) |
|
127 | |||
128 | /** |
||
129 | * Get the reader that provides the data. |
||
130 | * |
||
131 | * @return Reader |
||
132 | */ |
||
133 | 465 | public function getReader() |
|
137 | |||
138 | /** |
||
139 | * Get the CHM initial header. |
||
140 | * |
||
141 | * @return Header\ITSF |
||
142 | */ |
||
143 | 1 | public function getITSF() |
|
147 | |||
148 | /** |
||
149 | * Get the directory listing header. |
||
150 | * |
||
151 | * @return Header\ITSP |
||
152 | */ |
||
153 | public function getITSP() |
||
157 | |||
158 | /** |
||
159 | * Get an entry given its full path. |
||
160 | * |
||
161 | * @param string $path The full path (case sensitive) of the entry to look for. |
||
162 | * |
||
163 | * @return Entry|null |
||
164 | */ |
||
165 | 466 | public function getEntryByPath($path) |
|
169 | |||
170 | /** |
||
171 | * Get the entries contained in this CHM. |
||
172 | * |
||
173 | * @param int|null $type One or more Entry::TYPE_... values (defaults to Entry::TYPE_FILE | Entry::TYPE_DIRECTORY if null). |
||
174 | */ |
||
175 | 1 | public function getEntries($type = null) |
|
189 | |||
190 | /** |
||
191 | * Return a section given its index. |
||
192 | * |
||
193 | * @param int $i |
||
194 | * |
||
195 | * @return Section\Section|null |
||
196 | */ |
||
197 | 466 | public function getSectionByIndex($i) |
|
201 | |||
202 | /** |
||
203 | * Retrieve the list of the entries contained in this CHM. |
||
204 | * |
||
205 | * @throws Exception Throws an Exception in case of errors. |
||
206 | * |
||
207 | * @return Entry[] |
||
208 | */ |
||
209 | 1 | protected function retrieveEntryList() |
|
241 | |||
242 | /** |
||
243 | * Retrieve the list of the data sections contained in this CHM. |
||
244 | * |
||
245 | * @throws Exception Throws an Exception in case of errors. |
||
246 | */ |
||
247 | 1 | protected function retrieveSectionList() |
|
284 | |||
285 | /** |
||
286 | * Get the TOC of this CHM file (if available). |
||
287 | * |
||
288 | * @return SpecialEntry\TOC|null |
||
289 | */ |
||
290 | View Code Duplication | public function getTOC() |
|
305 | |||
306 | /** |
||
307 | * Get the index of this CHM file (if available). |
||
308 | * |
||
309 | * @return TOCIndex\Tree|null |
||
310 | */ |
||
311 | View Code Duplication | public function getIndex() |
|
326 | } |
||
327 |
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.