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 AbstractEntity 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 AbstractEntity, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
27 | abstract class AbstractEntity implements \ArrayAccess |
||
28 | { |
||
29 | private $AnnotationReader; |
||
30 | |||
31 | 321 | public function offsetExists($offset) |
|
40 | |||
41 | public function offsetSet($offset, $value) |
||
44 | |||
45 | 328 | public function offsetGet($offset) |
|
59 | |||
60 | public function offsetUnset($offset) |
||
63 | |||
64 | /** |
||
65 | * 引数の連想配列を元にプロパティを設定します. |
||
66 | * DBから取り出した連想配列を, プロパティへ設定する際に使用します. |
||
67 | * |
||
68 | * @param array $arrProps プロパティの情報を格納した連想配列 |
||
69 | * @param \ReflectionClass $parentClass 親のクラス. 本メソッドの内部的に使用します. |
||
70 | * @param string[] $excludeAttribute 除外したいフィールド名の配列 |
||
71 | */ |
||
72 | 243 | public function setPropertiesFromArray(array $arrProps, array $excludeAttribute = [], \ReflectionClass $parentClass = null) |
|
96 | |||
97 | /** |
||
98 | * Convert to associative array. |
||
99 | * |
||
100 | * Symfony Serializer Component is expensive, and hard to implementation. |
||
101 | * Use for encoder only. |
||
102 | * |
||
103 | * @param \ReflectionClass $parentClass parent class. Use internally of this method.. |
||
104 | * @param array $excludeAttribute Array of field names to exclusion. |
||
105 | * |
||
106 | * @return array |
||
107 | */ |
||
108 | 432 | public function toArray(array $excludeAttribute = ['__initializer__', '__cloner__', '__isInitialized__', 'AnnotationReader'], \ReflectionClass $parentClass = null) |
|
141 | |||
142 | /** |
||
143 | * Convert to associative array, and normalize to association properties. |
||
144 | * |
||
145 | * The type conversion such as: |
||
146 | * - Datetime :: W3C datetime format string |
||
147 | * - AbstractEntity :: associative array such as [id => value] |
||
148 | * - PersistentCollection :: associative array of [[id => value], [id => value], ...] |
||
149 | * |
||
150 | * @param array $excludeAttribute Array of field names to exclusion. |
||
151 | * |
||
152 | * @return array |
||
153 | */ |
||
154 | 3 | public function toNormalizedArray(array $excludeAttribute = ['__initializer__', '__cloner__', '__isInitialized__', 'AnnotationReader']) |
|
177 | |||
178 | /** |
||
179 | * Convert to JSON. |
||
180 | * |
||
181 | * @param array $excludeAttribute Array of field names to exclusion. |
||
182 | * |
||
183 | * @return string |
||
184 | */ |
||
185 | 1 | public function toJSON(array $excludeAttribute = ['__initializer__', '__cloner__', '__isInitialized__', 'AnnotationReader']) |
|
189 | |||
190 | /** |
||
191 | * Convert to XML. |
||
192 | * |
||
193 | * @param array $excludeAttribute Array of field names to exclusion. |
||
194 | * |
||
195 | * @return string |
||
196 | */ |
||
197 | 1 | public function toXML(array $excludeAttribute = ['__initializer__', '__cloner__', '__isInitialized__', 'AnnotationReader']) |
|
204 | |||
205 | /** |
||
206 | * コピー元のオブジェクトのフィールド名を指定して、同名のフィールドに値をコピー |
||
207 | * |
||
208 | * @param object $srcObject コピー元のオブジェクト |
||
209 | * @param string[] $excludeAttribute 除外したいフィールド名の配列 |
||
210 | * |
||
211 | * @return AbstractEntity |
||
212 | */ |
||
213 | 212 | public function copyProperties($srcObject, array $excludeAttribute = []) |
|
219 | |||
220 | /** |
||
221 | * Set AnnotationReader. |
||
222 | * |
||
223 | * @param Reader $Reader |
||
224 | * |
||
225 | * @return AbstractEntity |
||
226 | */ |
||
227 | 974 | public function setAnnotationReader(Reader $Reader) |
|
233 | |||
234 | /** |
||
235 | * Get AnnotationReader. |
||
236 | * |
||
237 | * @return Reader |
||
238 | */ |
||
239 | 3 | public function getAnnotationReader() |
|
247 | |||
248 | /** |
||
249 | * Convert to Entity of Identity value to associative array. |
||
250 | * |
||
251 | * @param AbstractEntity $Entity |
||
252 | * |
||
253 | * @return array associative array of [[id => value], [id => value], ...] |
||
254 | */ |
||
255 | 3 | public function getEntityIdentifierAsArray(AbstractEntity $Entity) |
|
275 | } |
||
276 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVar
assignment in line 1 and the$higher
assignment in line 2 are dead. The first because$myVar
is never used and the second because$higher
is always overwritten for every possible time line.