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 Store 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 Store, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
25 | class Store |
||
26 | { |
||
27 | /** |
||
28 | * @var MetadataFactory |
||
29 | */ |
||
30 | private $mf; |
||
31 | |||
32 | /** |
||
33 | * @var TypeFactory |
||
34 | */ |
||
35 | private $typeFactory; |
||
36 | |||
37 | /** |
||
38 | * The storage layer manager. |
||
39 | * Retrieves the appropriate persister and search client for handling records. |
||
40 | * |
||
41 | * @var StorageLayerManager |
||
42 | */ |
||
43 | private $storageManager; |
||
44 | |||
45 | /** |
||
46 | * Contains all models currently loaded in memory. |
||
47 | * |
||
48 | * @var Cache |
||
49 | */ |
||
50 | private $cache; |
||
51 | |||
52 | /** |
||
53 | * The event dispatcher for firing model lifecycle events. |
||
54 | * |
||
55 | * @var EventDispatcher |
||
56 | */ |
||
57 | private $dispatcher; |
||
58 | |||
59 | /** |
||
60 | * Constructor. |
||
61 | * |
||
62 | * @param MetadataFactory $mf |
||
63 | * @param StorageLayerManager $storageManager |
||
64 | * @param TypeFactory $typeFactory |
||
65 | * @param EventDispatcher $dispatcher |
||
66 | */ |
||
67 | public function __construct(MetadataFactory $mf, StorageLayerManager $storageManager, TypeFactory $typeFactory, EventDispatcher $dispatcher) |
||
75 | |||
76 | /** |
||
77 | * Finds a single record from the persistence layer, by type and id. |
||
78 | * Will return a Model object if found, or throw an exception if not. |
||
79 | * |
||
80 | * @api |
||
81 | * @param string $typeKey The model type. |
||
82 | * @param string $identifier The model id. |
||
83 | * @return Model |
||
84 | */ |
||
85 | public function find($typeKey, $identifier) |
||
93 | |||
94 | /** |
||
95 | * Returns the available type keys from the MetadataFactory |
||
96 | * |
||
97 | * @return array |
||
98 | */ |
||
99 | public function getModelTypes() |
||
103 | |||
104 | /** |
||
105 | * Finds all records (or filtered by specific identifiers) for a type. |
||
106 | * |
||
107 | * @todo Add sorting and pagination (limit/skip). |
||
108 | * @todo Handle find all with identifiers. |
||
109 | * @param string $typeKey The model type. |
||
110 | * @param array $idenitifiers The model identifiers (optional). |
||
|
|||
111 | * @return Collections\Collection |
||
112 | */ |
||
113 | public function findAll($typeKey, array $identifiers = []) |
||
123 | |||
124 | /** |
||
125 | * Queries records based on a provided set of criteria. |
||
126 | * |
||
127 | * @param string $typeKey The model type. |
||
128 | * @param array $criteria The query criteria. |
||
129 | * @param array $fields Fields to include/exclude. |
||
130 | * @param array $sort The sort criteria. |
||
131 | * @param int $offset The starting offset, aka the number of Models to skip. |
||
132 | * @param int $limit The number of Models to limit. |
||
133 | * @return Collections\Collection |
||
134 | */ |
||
135 | public function findQuery($typeKey, array $criteria, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
145 | |||
146 | /** |
||
147 | * Searches for records (via the search layer) for a specific type, attribute, and value. |
||
148 | * Uses the autocomplete logic to fullfill the request. |
||
149 | * |
||
150 | * @todo Determine if full models should be return, or only specific fields. |
||
151 | * Autocompleters needs to be fast. If only specific fields are returned, do we need to exclude nulls in serialization? |
||
152 | * @todo Is search enabled for all models, by default, where everything is stored? |
||
153 | * |
||
154 | * @param string $typeKey |
||
155 | * @param string $attributeKey |
||
156 | * @param string $searchValue |
||
157 | * @return Collections\Collection |
||
158 | */ |
||
159 | public function searchAutocomplete($typeKey, $attributeKey, $searchValue) |
||
167 | |||
168 | /** |
||
169 | * Creates a new record. |
||
170 | * The model will not be comitted to the persistence layer until $model->save() is called. |
||
171 | * |
||
172 | * @api |
||
173 | * @param string $typeKey The model type. |
||
174 | * @param string|null $identifier The model identifier. Generally should be null unless client-side id generation is in place. |
||
175 | * @return Model |
||
176 | */ |
||
177 | public function create($typeKey, $identifier = null) |
||
184 | |||
185 | /** |
||
186 | * Deletes a model. |
||
187 | * The moel will be immediately deleted once retrieved. |
||
188 | * |
||
189 | * @api |
||
190 | * @param string $typeKey The model type. |
||
191 | * @param string|null $identifier The model identifier. |
||
192 | * @return Model |
||
193 | */ |
||
194 | public function delete($typeKey, $identifier) |
||
199 | |||
200 | /** |
||
201 | * Retrieves a Record object from the persistence layer. |
||
202 | * |
||
203 | * @param string $typeKey The model type. |
||
204 | * @param string $identifier The model identifier. |
||
205 | * @return Record |
||
206 | * @throws StoreException If the record cannot be found. |
||
207 | */ |
||
208 | public function retrieveRecord($typeKey, $identifier) |
||
217 | |||
218 | /** |
||
219 | * Retrieves multiple Record objects from the persistence layer. |
||
220 | * |
||
221 | * @todo Implement sorting and pagination (limit/skip). |
||
222 | * @param string $typeKey The model type. |
||
223 | * @param array $identifiers The model identifier. |
||
224 | * @return Record[] |
||
225 | */ |
||
226 | public function retrieveRecords($typeKey, array $identifiers) |
||
231 | |||
232 | /** |
||
233 | * Retrieves multiple Record objects from the persistence layer for an inverse relationship. |
||
234 | * |
||
235 | * @todo Need to find a way to query all inverse at the same time for a findAll query, as it's queried multiple times. |
||
236 | * @param string $ownerTypeKey |
||
237 | * @param string $relTypeKey |
||
238 | * @param array $identifiers |
||
239 | * @param string $inverseField |
||
240 | * @return Record[] |
||
241 | */ |
||
242 | public function retrieveInverseRecords($ownerTypeKey, $relTypeKey, array $identifiers, $inverseField) |
||
253 | |||
254 | /** |
||
255 | * Loads/creates a model from a persistence layer Record. |
||
256 | * |
||
257 | * @param string $typeKey The model type. |
||
258 | * @param Record $record The persistence layer record. |
||
259 | * @return Model |
||
260 | */ |
||
261 | protected function loadModel($typeKey, Record $record) |
||
275 | |||
276 | /** |
||
277 | * Loads/creates multiple models from persistence layer Records. |
||
278 | * |
||
279 | * @param string $typeKey The model type. |
||
280 | * @param Record[] $records The persistence layer records. |
||
281 | * @return Model[] |
||
282 | */ |
||
283 | protected function loadModels($typeKey, array $records) |
||
291 | |||
292 | /** |
||
293 | * Dispatches a model lifecycle event via the event dispatcher. |
||
294 | * |
||
295 | * @param string $eventName |
||
296 | * @param Model $model |
||
297 | * @return self |
||
298 | */ |
||
299 | protected function dispatchLifecycleEvent($eventName, Model $model) |
||
305 | |||
306 | /** |
||
307 | * Creates a new Model instance. |
||
308 | * Will not be persisted until $model->save() is called. |
||
309 | * |
||
310 | * @param string $typeKey The model type. |
||
311 | * @param string $identifier The model identifier. |
||
312 | * @return Model |
||
313 | */ |
||
314 | protected function createModel($typeKey, $identifier) |
||
328 | |||
329 | /** |
||
330 | * Loads a has-one model proxy. |
||
331 | * |
||
332 | * @param string $relatedTypeKey |
||
333 | * @param string $identifier |
||
334 | * @return Model |
||
335 | */ |
||
336 | public function loadProxyModel($relatedTypeKey, $identifier) |
||
348 | |||
349 | /** |
||
350 | * Loads an Embed model |
||
351 | * |
||
352 | * @param EmbedMetadata $embedMeta |
||
353 | * @param array $embed |
||
354 | * @return Embed |
||
355 | */ |
||
356 | public function loadEmbed(EmbedMetadata $embedMeta, array $embed) |
||
360 | |||
361 | /** |
||
362 | * Loads a has-many inverse model collection. |
||
363 | * |
||
364 | * @param RelationshipMetadata $relMeta |
||
365 | * @param Model $owner |
||
366 | * @return Collections\InverseCollection |
||
367 | */ |
||
368 | public function createInverseCollection(RelationshipMetadata $relMeta, Model $owner) |
||
373 | |||
374 | /** |
||
375 | * Loads a has-many embed collection. |
||
376 | * |
||
377 | * @param EmbeddedPropMetadata $relMeta |
||
378 | * @param array|null $embedDocs |
||
379 | * @return Collections\EmbedCollection |
||
380 | */ |
||
381 | public function createEmbedCollection(EmbeddedPropMetadata $embededPropMeta, array $embedDocs = null) |
||
397 | |||
398 | /** |
||
399 | * Loads a has-many model collection. |
||
400 | * |
||
401 | * @param RelationshipMetadata $relMeta |
||
402 | * @param array|null $references |
||
403 | * @return Collections\Collection |
||
404 | */ |
||
405 | public function createCollection(RelationshipMetadata $relMeta, array $references = null) |
||
420 | |||
421 | /** |
||
422 | * Loads/fills a collection of empty (unloaded) models with data from the persistence layer. |
||
423 | * |
||
424 | * @param Collections\AbstractCollection $collection |
||
425 | * @return Model[] |
||
426 | */ |
||
427 | public function loadCollection(Collections\AbstractCollection $collection) |
||
450 | |||
451 | /** |
||
452 | * Commits a model by persisting it to the database. |
||
453 | * |
||
454 | * @todo Eventually we'll want to schedule models and allow for mutiple commits, flushes, etc. |
||
455 | * @todo Will need to handle cascade saving of new or modified relationships?? |
||
456 | * @param Model $model The model to commit. |
||
457 | * @return Model |
||
458 | */ |
||
459 | public function commit(Model $model) |
||
485 | |||
486 | /** |
||
487 | * Performs a Model creation commit and persists to the database. |
||
488 | * |
||
489 | * @param Model $model |
||
490 | * @return Model |
||
491 | */ |
||
492 | View Code Duplication | private function doCommitCreate(Model $model) |
|
504 | |||
505 | /** |
||
506 | * Performs a Model delete commit and persists to the database. |
||
507 | * |
||
508 | * @param Model $model |
||
509 | * @return Model |
||
510 | */ |
||
511 | View Code Duplication | private function doCommitDelete(Model $model) |
|
521 | |||
522 | /** |
||
523 | * Performs a Model update commit and persists to the database. |
||
524 | * |
||
525 | * @param Model $model |
||
526 | * @return Model |
||
527 | */ |
||
528 | View Code Duplication | private function doCommitUpdate(Model $model) |
|
539 | |||
540 | /** |
||
541 | * Validates that an embed name/type can be set to an owning embed metadata type. |
||
542 | * |
||
543 | * @param EmbedMetadata $owningMeta The metadata the type will be added to. |
||
544 | * @param string $nameToCheck The name to check. |
||
545 | * @return self |
||
546 | * @throws StoreException If the type to add is not supported. |
||
547 | */ |
||
548 | public function validateEmbedSet(EmbedMetadata $owningMeta, $nameToCheck) |
||
555 | |||
556 | /** |
||
557 | * Validates that a model type can be set to an owning metadata type. |
||
558 | * |
||
559 | * @param EntityMetadata $owningMeta The metadata the type will be added to. |
||
560 | * @param string $typeToAdd The type to add. |
||
561 | * @return self |
||
562 | * @throws StoreException If the type to add is not supported. |
||
563 | */ |
||
564 | public function validateRelationshipSet(EntityMetadata $owningMeta, $typeToAdd) |
||
576 | |||
577 | /** |
||
578 | * Converts an attribute value to the proper Modlr data type. |
||
579 | * |
||
580 | * @param string $dataType The data type, such as string, integer, boolean, etc. |
||
581 | * @param mixed $value The value to convert. |
||
582 | * @return mixed |
||
583 | */ |
||
584 | public function convertAttributeValue($dataType, $value) |
||
588 | |||
589 | /** |
||
590 | * Determines if a model is eligible for commit. |
||
591 | * |
||
592 | * @todo Does delete need to be here? |
||
593 | * @param Model $model |
||
594 | * @return bool |
||
595 | */ |
||
596 | protected function shouldCommit(Model $model) |
||
601 | |||
602 | /** |
||
603 | * Determines the persister to use for the provided model key. |
||
604 | * |
||
605 | * @param string $typeKey The model type key. |
||
606 | * @return PersisterInterface |
||
607 | */ |
||
608 | protected function getPersisterFor($typeKey) |
||
613 | |||
614 | /** |
||
615 | * Generates a new identifier value for a model type. |
||
616 | * |
||
617 | * @param string $typeKey The model type. |
||
618 | * @return string |
||
619 | */ |
||
620 | protected function generateIdentifier($typeKey) |
||
624 | |||
625 | /** |
||
626 | * Converts the id value to a normalized string. |
||
627 | * |
||
628 | * @param mixed $identenfier The identifier to convert. |
||
629 | * @return string |
||
630 | */ |
||
631 | protected function convertId($identifier) |
||
635 | |||
636 | /** |
||
637 | * Gets the metadata for a model type. |
||
638 | * |
||
639 | * @param string $typeKey The model type. |
||
640 | * @return EntityMetadata |
||
641 | */ |
||
642 | public function getMetadataForType($typeKey) |
||
646 | |||
647 | /** |
||
648 | * Gets the metadata for a relationship. |
||
649 | * |
||
650 | * @param RelationshipMetadata $relMeta The relationship metadata. |
||
651 | * @return EntityMetadata |
||
652 | */ |
||
653 | public function getMetadataForRelationship(RelationshipMetadata $relMeta) |
||
657 | |||
658 | /** |
||
659 | * Determines if an array is sequential. |
||
660 | * |
||
661 | * @param array $arr |
||
662 | * @return bool |
||
663 | */ |
||
664 | protected function isSequentialArray(array $arr) |
||
671 | } |
||
672 |
This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.
Consider the following example. The parameter
$ireland
is not defined by the methodfinale(...)
.The most likely cause is that the parameter was changed, but the annotation was not.