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.