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 AnnotationDriver 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 AnnotationDriver, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
27 | class AnnotationDriver extends DoctrineAnnotationDriver |
||
28 | { |
||
29 | public $reader; |
||
30 | protected $eventDispatcher; |
||
31 | protected $widgetHelper; |
||
32 | protected $paths; |
||
33 | |||
34 | /** |
||
35 | * construct. |
||
36 | * |
||
37 | * @param Reader $reader |
||
38 | * @param EventDispatcherInterface $eventDispatcher |
||
39 | * @param WidgetHelper $widgetHelper |
||
40 | * @param array $paths The paths where to search about Entities |
||
41 | */ |
||
42 | public function __construct(Reader $reader, EventDispatcherInterface $eventDispatcher, $widgetHelper, $paths) |
||
49 | |||
50 | /** |
||
51 | * Get all class names. |
||
52 | * |
||
53 | * @return string[] |
||
54 | */ |
||
55 | public function getAllClassNames() |
||
91 | |||
92 | /** |
||
93 | * Parse the given Class to find some annotations related to BusinessEntities. |
||
94 | */ |
||
95 | public function parse(\ReflectionClass $class) |
||
160 | |||
161 | /** |
||
162 | * load business properties from ReflectionClass. |
||
163 | * |
||
164 | * @return array |
||
165 | **/ |
||
166 | protected function loadBusinessProperties(\ReflectionClass $class) |
||
167 | { |
||
168 | $businessProperties = []; |
||
169 | $properties = $class->getProperties(); |
||
170 | $traits = $class->getTraits(); |
||
171 | $className = $class->getName(); |
||
172 | // if the class is translatable, then parse annotations on it's translation class |
||
173 | if (array_key_exists(Translatable::class, $traits)) { |
||
174 | $translation = new \ReflectionClass($className::getTranslationEntityClass()); |
||
175 | $translationProperties = $translation->getProperties(); |
||
176 | $properties = array_merge($properties, $translationProperties); |
||
177 | } |
||
178 | |||
179 | View Code Duplication | foreach ($properties as $property) { |
|
180 | $annotations = $this->reader->getPropertyAnnotations($property); |
||
181 | foreach ($annotations as $key => $annotationObj) { |
||
182 | if ($annotationObj instanceof BusinessProperty && !in_array($class, $businessProperties)) { |
||
183 | if (!$annotations[$key]->getTypes()) { |
||
184 | $message = $class->name.':$'.$property->name.'" field'; |
||
185 | throw AnnotationException::requiredError('type', 'BusinessProperty annotation', $message, 'array or string'); |
||
186 | } |
||
187 | foreach ($annotations[$key]->getTypes() as $type) { |
||
188 | $businessProperties[$type][] = $property->name; |
||
189 | } |
||
190 | } |
||
191 | } |
||
192 | } |
||
193 | // we load business properties of parents recursively |
||
194 | // because they are defined by an annotation not by the property type(private, protected, public) |
||
195 | $parentClass = $class->getParentClass(); |
||
196 | if ($parentClass) { |
||
197 | //load parent properties recursively |
||
198 | $parentProperties = $this->loadBusinessProperties(new \ReflectionClass($parentClass->getName())); |
||
199 | foreach ($parentProperties as $key => $parentProperty) { |
||
200 | if (array_key_exists($key, $businessProperties)) { |
||
201 | //if parent and current have a same business property type we merge the properties and remove |
||
202 | //duplicates if properties are the same; |
||
203 | $businessProperties[$key] = array_unique(array_merge($parentProperty, $businessProperties[$key])); |
||
204 | } else { |
||
205 | //else we had a business property type for the parent properties |
||
206 | $businessProperties[$key] = $parentProperty; |
||
207 | } |
||
208 | } |
||
209 | } |
||
210 | |||
211 | return $businessProperties; |
||
212 | } |
||
213 | |||
214 | /** |
||
215 | * Load receiver properties and NotBlank constraints from ReflectionClass. |
||
216 | * |
||
217 | * @param \ReflectionClass $class |
||
218 | * |
||
219 | * @throws AnnotationException |
||
220 | * |
||
221 | * @return array |
||
222 | */ |
||
223 | protected function loadReceiverProperties(\ReflectionClass $class) |
||
282 | } |
||
283 |
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.