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 Blameable 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 Blameable, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
36 | final class Blameable extends Nette\Object |
||
37 | 1 | { |
|
38 | /** |
||
39 | * Define class name |
||
40 | */ |
||
41 | const CLASS_NAME = __CLASS__; |
||
42 | |||
43 | /** |
||
44 | * Annotation field is blameable |
||
45 | */ |
||
46 | const EXTENSION_ANNOTATION = 'IPub\DoctrineBlameable\Mapping\Annotation\Blameable'; |
||
47 | |||
48 | /** |
||
49 | * @var Common\Persistence\ObjectManager |
||
50 | */ |
||
51 | private $objectManager; |
||
52 | |||
53 | /** |
||
54 | * @var DoctrineBlameable\Configuration |
||
55 | */ |
||
56 | private $configuration; |
||
57 | |||
58 | /** |
||
59 | * List of cached object configurations |
||
60 | * |
||
61 | * @var array |
||
62 | */ |
||
63 | private static $objectConfigurations = []; |
||
64 | |||
65 | /** |
||
66 | * List of types which are valid for blame |
||
67 | * |
||
68 | * @var array |
||
69 | */ |
||
70 | private $validTypes = [ |
||
71 | 'one', |
||
72 | 'string', |
||
73 | 'int', |
||
74 | ]; |
||
75 | |||
76 | /** |
||
77 | * @param DoctrineBlameable\Configuration $configuration |
||
78 | * @param Common\Persistence\ObjectManager $objectManager |
||
79 | */ |
||
80 | public function __construct( |
||
87 | |||
88 | /** |
||
89 | * @param ORM\Mapping\ClassMetadata $classMetadata |
||
90 | */ |
||
91 | public function loadMetadataForObjectClass(ORM\Mapping\ClassMetadata $classMetadata) |
||
143 | |||
144 | /** |
||
145 | * @param ORM\Mapping\ClassMetadata $metadata |
||
146 | * @param array $config |
||
147 | * |
||
148 | * @return array |
||
149 | * |
||
150 | * @throws Exceptions\InvalidMappingException |
||
151 | * @throws ORM\Mapping\MappingException |
||
152 | */ |
||
153 | private function readExtendedMetadata(ORM\Mapping\ClassMetadata $metadata, array $config) |
||
154 | { |
||
155 | 1 | $class = $metadata->getReflectionClass(); |
|
156 | |||
157 | // Create doctrine annotation reader |
||
158 | 1 | $reader = $this->getDefaultAnnotationReader(); |
|
159 | |||
160 | // Property annotations |
||
161 | 1 | foreach ($class->getProperties() as $property) { |
|
162 | 1 | if ($metadata->isMappedSuperclass && $property->isPrivate() === FALSE || |
|
163 | 1 | $metadata->isInheritedField($property->getName()) || |
|
164 | 1 | isset($metadata->associationMappings[$property->getName()]['inherited']) |
|
165 | 1 | ) { |
|
166 | continue; |
||
167 | } |
||
168 | |||
169 | /** @var Mapping\Annotation\Blameable $blameable */ |
||
170 | 1 | if ($blameable = $reader->getPropertyAnnotation($property, self::EXTENSION_ANNOTATION)) { |
|
|
|||
171 | 1 | $field = $property->getName(); |
|
172 | |||
173 | // No map field nor association |
||
174 | 1 | if ($metadata->hasField($field) === FALSE && $metadata->hasAssociation($field) === FALSE && $this->configuration->useLazyAssociation() === FALSE) { |
|
175 | 1 | if ($this->configuration->automapField) { |
|
176 | 1 | if ($this->configuration->automapWithAssociation()) { |
|
177 | $entityMap = [ |
||
178 | 1 | 'targetEntity' => $this->configuration->userEntity, |
|
179 | 1 | 'fieldName' => $field, |
|
180 | 'joinColumns' => [ |
||
181 | [ |
||
182 | 1 | 'onDelete' => 'SET NULL', |
|
183 | ] |
||
184 | 1 | ] |
|
185 | 1 | ]; |
|
186 | |||
187 | 1 | View Code Duplication | if (isset($blameable->association['column']) && $blameable->association['column'] !== NULL) { |
188 | $entityMap['joinColumns'][0]['name'] = $blameable->columnName; |
||
189 | } |
||
190 | |||
191 | 1 | View Code Duplication | if (isset($blameable->association['referencedColumn']) && $blameable->association['referencedColumn'] !== NULL) { |
192 | $entityMap['joinColumns'][0]['referencedColumnName'] = $blameable->referencedColumnName; |
||
193 | } |
||
194 | |||
195 | 1 | $metadata->mapManyToOne($entityMap); |
|
196 | |||
197 | 1 | } else if ($this->configuration->automapWithField()) { |
|
198 | 1 | $metadata->mapField([ |
|
199 | 1 | 'fieldName' => $field, |
|
200 | 1 | 'type' => 'string', |
|
201 | 1 | 'nullable' => TRUE, |
|
202 | 1 | ]); |
|
203 | |||
204 | 1 | } else { |
|
205 | throw new Exceptions\InvalidMappingException("Unable to find blameable [{$field}] as mapped property in entity - {$metadata->getName()}"); |
||
206 | } |
||
207 | |||
208 | 1 | } else { |
|
209 | throw new Exceptions\InvalidMappingException("Unable to find blameable [{$field}] as mapped property in entity - {$metadata->getName()}"); |
||
210 | } |
||
211 | 1 | } |
|
212 | |||
213 | 1 | if ($metadata->hasField($field)) { |
|
214 | 1 | View Code Duplication | if (!$this->isValidField($metadata, $field) && $this->configuration->useLazyAssociation() === FALSE) { |
215 | throw new Exceptions\InvalidMappingException("Field - [{$field}] type is not valid and must be 'string' or a one-to-many relation in class - {$metadata->getName()}"); |
||
216 | } |
||
217 | |||
218 | 1 | } else if ($metadata->hasAssociation($field)) { |
|
219 | // association |
||
220 | 1 | View Code Duplication | if ($metadata->isSingleValuedAssociation($field) === FALSE && $this->configuration->useLazyAssociation() === FALSE) { |
221 | throw new Exceptions\InvalidMappingException("Association - [{$field}] is not valid, it must be a one-to-many relation or a string field - {$metadata->getName()}"); |
||
222 | } |
||
223 | 1 | } |
|
224 | |||
225 | // Check for valid events |
||
226 | 1 | if (!in_array($blameable->on, ['update', 'create', 'change', 'delete'])) { |
|
227 | throw new Exceptions\InvalidMappingException("Field - [{$field}] trigger 'on' is not one of [update, create, change] in class - {$metadata->getName()}"); |
||
228 | } |
||
229 | |||
230 | 1 | if ($blameable->on === 'change') { |
|
231 | 1 | if (!isset($blameable->field)) { |
|
232 | throw new Exceptions\InvalidMappingException("Missing parameters on property - {$field}, field must be set on [change] trigger in class - {$metadata->getName()}"); |
||
233 | } |
||
234 | |||
235 | 1 | if (is_array($blameable->field) && isset($blameable->value)) { |
|
236 | throw new Exceptions\InvalidMappingException("Blameable extension does not support multiple value changeset detection yet."); |
||
237 | } |
||
238 | |||
239 | $field = [ |
||
240 | 1 | 'field' => $field, |
|
241 | 1 | 'trackedField' => $blameable->field, |
|
242 | 1 | 'value' => $blameable->value, |
|
243 | 1 | ]; |
|
244 | 1 | } |
|
245 | |||
246 | // properties are unique and mapper checks that, no risk here |
||
247 | 1 | $config[$blameable->on][] = $field; |
|
248 | 1 | } |
|
249 | 1 | } |
|
250 | |||
251 | 1 | return $config; |
|
252 | } |
||
253 | |||
254 | /** |
||
255 | * Get the configuration for specific object class |
||
256 | * if cache driver is present it scans it also |
||
257 | * |
||
258 | * @param string $class |
||
259 | * |
||
260 | * @return array |
||
261 | */ |
||
262 | public function getObjectConfigurations($class) |
||
303 | |||
304 | /** |
||
305 | * Create default annotation reader for extensions |
||
306 | * |
||
307 | * @return Common\Annotations\AnnotationReader |
||
308 | */ |
||
309 | private function getDefaultAnnotationReader() |
||
321 | |||
322 | /** |
||
323 | * Checks if $field type is valid |
||
324 | * |
||
325 | * @param object $meta |
||
326 | * @param string $field |
||
327 | * |
||
328 | * @return boolean |
||
329 | */ |
||
330 | private function isValidField($meta, $field) |
||
336 | |||
337 | /** |
||
338 | * Get the cache id |
||
339 | * |
||
340 | * @param string $className |
||
341 | * |
||
342 | * @return string |
||
343 | */ |
||
344 | private static function getCacheId($className) |
||
348 | |||
349 | } |
||
350 |
This check looks for function or method calls that always return null and whose return value is assigned to a variable.
The method
getObject()
can return nothing but null, so it makes no sense to assign that value to a variable.The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.