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 |
||
26 | class Store |
||
27 | { |
||
28 | /** |
||
29 | * @var MetadataFactory |
||
30 | */ |
||
31 | private $mf; |
||
32 | |||
33 | /** |
||
34 | * @var TypeFactory |
||
35 | */ |
||
36 | private $typeFactory; |
||
37 | |||
38 | /** |
||
39 | * The storage layer manager. |
||
40 | * Retrieves the appropriate persister and search client for handling records. |
||
41 | * |
||
42 | * @var StorageLayerManager |
||
43 | */ |
||
44 | private $storageManager; |
||
45 | |||
46 | /** |
||
47 | * Contains all models currently loaded in memory. |
||
48 | * |
||
49 | * @var Cache |
||
50 | */ |
||
51 | private $cache; |
||
52 | |||
53 | /** |
||
54 | * The event dispatcher for firing model lifecycle events. |
||
55 | * |
||
56 | * @var EventDispatcher |
||
57 | */ |
||
58 | private $dispatcher; |
||
59 | |||
60 | /** |
||
61 | * Constructor. |
||
62 | * |
||
63 | * @param MetadataFactory $mf |
||
64 | * @param StorageLayerManager $storageManager |
||
65 | * @param TypeFactory $typeFactory |
||
66 | * @param EventDispatcher $dispatcher |
||
67 | */ |
||
68 | public function __construct(MetadataFactory $mf, StorageLayerManager $storageManager, TypeFactory $typeFactory, EventDispatcher $dispatcher) |
||
76 | |||
77 | /** |
||
78 | * Finds a single record from the persistence layer, by type and id. |
||
79 | * Will return a Model object if found, or throw an exception if not. |
||
80 | * |
||
81 | * @api |
||
82 | * @param string $typeKey The model type. |
||
83 | * @param string $identifier The model id. |
||
84 | * @return Model |
||
85 | */ |
||
86 | public function find($typeKey, $identifier) |
||
94 | |||
95 | /** |
||
96 | * Returns the available type keys from the MetadataFactory |
||
97 | * |
||
98 | * @return array |
||
99 | */ |
||
100 | public function getModelTypes() |
||
104 | |||
105 | /** |
||
106 | * Finds all records (or filtered by specific identifiers) for a type. |
||
107 | * |
||
108 | * @todo Add sorting and pagination (limit/skip). |
||
109 | * @todo Handle find all with identifiers. |
||
110 | * @param string $typeKey The model type. |
||
111 | * @param array $idenitifiers The model identifiers (optional). |
||
|
|||
112 | * @param array $fields |
||
113 | * @param array $sort |
||
114 | * @param int $offset |
||
115 | * @param int $limit |
||
116 | * @return Collections\Collection |
||
117 | */ |
||
118 | public function findAll($typeKey, array $identifiers = [], array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
128 | |||
129 | /** |
||
130 | * Queries records based on a provided set of criteria. |
||
131 | * |
||
132 | * @param string $typeKey The model type. |
||
133 | * @param array $criteria The query criteria. |
||
134 | * @param array $fields Fields to include/exclude. |
||
135 | * @param array $sort The sort criteria. |
||
136 | * @param int $offset The starting offset, aka the number of Models to skip. |
||
137 | * @param int $limit The number of Models to limit. |
||
138 | * @return Collections\Collection |
||
139 | */ |
||
140 | public function findQuery($typeKey, array $criteria, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
151 | |||
152 | /** |
||
153 | * Gets the model memory cache (identity map). |
||
154 | * |
||
155 | * @return Cache |
||
156 | */ |
||
157 | public function getModelCache() |
||
161 | |||
162 | /** |
||
163 | * Searches for records (via the search layer) for a specific type, attribute, and value. |
||
164 | * Uses the autocomplete logic to fullfill the request. |
||
165 | * |
||
166 | * @todo Determine if full models should be return, or only specific fields. |
||
167 | * Autocompleters needs to be fast. If only specific fields are returned, do we need to exclude nulls in serialization? |
||
168 | * @todo Is search enabled for all models, by default, where everything is stored? |
||
169 | * |
||
170 | * @param string $typeKey |
||
171 | * @param string $attributeKey |
||
172 | * @param string $searchValue |
||
173 | * @return Collections\Collection |
||
174 | */ |
||
175 | public function searchAutocomplete($typeKey, $attributeKey, $searchValue) |
||
183 | |||
184 | /** |
||
185 | * Creates a new record. |
||
186 | * The model will not be comitted to the persistence layer until $model->save() is called. |
||
187 | * |
||
188 | * @api |
||
189 | * @param string $typeKey The model type. |
||
190 | * @param string|null $identifier The model identifier. Generally should be null unless client-side id generation is in place. |
||
191 | * @return Model |
||
192 | */ |
||
193 | public function create($typeKey, $identifier = null) |
||
200 | |||
201 | /** |
||
202 | * Deletes a model. |
||
203 | * The moel will be immediately deleted once retrieved. |
||
204 | * |
||
205 | * @api |
||
206 | * @param string $typeKey The model type. |
||
207 | * @param string|null $identifier The model identifier. |
||
208 | * @return Model |
||
209 | */ |
||
210 | public function delete($typeKey, $identifier) |
||
215 | |||
216 | /** |
||
217 | * Retrieves a RecordSet object from the persistence layer. |
||
218 | * |
||
219 | * @param string $typeKey The model type. |
||
220 | * @param string $identifier The model identifier. |
||
221 | * @return array |
||
222 | * @throws StoreException If the record cannot be found. |
||
223 | */ |
||
224 | public function retrieveRecord($typeKey, $identifier) |
||
233 | |||
234 | /** |
||
235 | * Retrieves multiple Record objects from the persistence layer. |
||
236 | * |
||
237 | * @todo Implement sorting and pagination (limit/skip). |
||
238 | * @param string $typeKey The model type. |
||
239 | * @param array $identifiers The model identifier. |
||
240 | * @param array $fields |
||
241 | * @param array $sort |
||
242 | * @param int $offset |
||
243 | * @param int $limit |
||
244 | * @return RecordSetInterface |
||
245 | */ |
||
246 | public function retrieveRecords($typeKey, array $identifiers, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
251 | |||
252 | /** |
||
253 | * Retrieves multiple Record objects from the persistence layer for an inverse relationship. |
||
254 | * |
||
255 | * @todo Need to find a way to query all inverse at the same time for a findAll query, as it's queried multiple times. |
||
256 | * @param string $ownerTypeKey |
||
257 | * @param string $relTypeKey |
||
258 | * @param array $identifiers |
||
259 | * @param string $inverseField |
||
260 | * @return RecordSetInterface |
||
261 | */ |
||
262 | public function retrieveInverseRecords($ownerTypeKey, $relTypeKey, array $identifiers, $inverseField) |
||
273 | |||
274 | /** |
||
275 | * Loads/creates a model from a persistence layer Record. |
||
276 | * |
||
277 | * @param string $typeKey The model type. |
||
278 | * @param array $record The persistence layer record. |
||
279 | * @return Model |
||
280 | */ |
||
281 | protected function loadModel($typeKey, array $record) |
||
295 | |||
296 | /** |
||
297 | * Loads/creates multiple models from persistence layer Records. |
||
298 | * |
||
299 | * @param string $typeKey The model type. |
||
300 | * @param RecordSetInterface $records The persistence layer records. |
||
301 | * @return Model[] |
||
302 | */ |
||
303 | protected function loadModels($typeKey, RecordSetInterface $records) |
||
311 | |||
312 | /** |
||
313 | * Dispatches a model lifecycle event via the event dispatcher. |
||
314 | * |
||
315 | * @param string $eventName |
||
316 | * @param Model $model |
||
317 | * @return self |
||
318 | */ |
||
319 | protected function dispatchLifecycleEvent($eventName, Model $model) |
||
325 | |||
326 | /** |
||
327 | * Creates a new Model instance. |
||
328 | * Will not be persisted until $model->save() is called. |
||
329 | * |
||
330 | * @param string $typeKey The model type. |
||
331 | * @param string $identifier The model identifier. |
||
332 | * @return Model |
||
333 | */ |
||
334 | protected function createModel($typeKey, $identifier) |
||
348 | |||
349 | /** |
||
350 | * Loads a has-one model proxy. |
||
351 | * |
||
352 | * @param string $relatedTypeKey |
||
353 | * @param string $identifier |
||
354 | * @return Model |
||
355 | */ |
||
356 | public function loadProxyModel($relatedTypeKey, $identifier) |
||
368 | |||
369 | /** |
||
370 | * Loads an Embed model |
||
371 | * |
||
372 | * @param EmbedMetadata $embedMeta |
||
373 | * @param array $embed |
||
374 | * @return Embed |
||
375 | */ |
||
376 | public function loadEmbed(EmbedMetadata $embedMeta, array $embed) |
||
380 | |||
381 | /** |
||
382 | * Loads a has-many inverse model collection. |
||
383 | * |
||
384 | * @param RelationshipMetadata $relMeta |
||
385 | * @param Model $owner |
||
386 | * @return Collections\InverseCollection |
||
387 | */ |
||
388 | public function createInverseCollection(RelationshipMetadata $relMeta, Model $owner) |
||
393 | |||
394 | /** |
||
395 | * Loads a has-many embed collection. |
||
396 | * |
||
397 | * @param EmbeddedPropMetadata $relMeta |
||
398 | * @param array|null $embedDocs |
||
399 | * @return Collections\EmbedCollection |
||
400 | */ |
||
401 | public function createEmbedCollection(EmbeddedPropMetadata $embededPropMeta, array $embedDocs = null) |
||
417 | |||
418 | /** |
||
419 | * Loads a has-many model collection. |
||
420 | * |
||
421 | * @param RelationshipMetadata $relMeta |
||
422 | * @param array|null $references |
||
423 | * @return Collections\Collection |
||
424 | */ |
||
425 | public function createCollection(RelationshipMetadata $relMeta, array $references = null) |
||
440 | |||
441 | /** |
||
442 | * Determines the persister to use for the provided model key. |
||
443 | * |
||
444 | * @param string $typeKey The model type key. |
||
445 | * @return PersisterInterface |
||
446 | */ |
||
447 | public function getPersisterFor($typeKey) |
||
452 | |||
453 | /** |
||
454 | * Loads/fills a collection of empty (unloaded) models with data from the persistence layer. |
||
455 | * |
||
456 | * @param Collections\AbstractCollection $collection |
||
457 | * @return Model[] |
||
458 | */ |
||
459 | public function loadCollection(Collections\AbstractCollection $collection) |
||
482 | |||
483 | /** |
||
484 | * Commits a model by persisting it to the database. |
||
485 | * |
||
486 | * @todo Eventually we'll want to schedule models and allow for mutiple commits, flushes, etc. |
||
487 | * @todo Will need to handle cascade saving of new or modified relationships?? |
||
488 | * @param Model $model The model to commit. |
||
489 | * @return Model |
||
490 | */ |
||
491 | public function commit(Model $model) |
||
517 | |||
518 | /** |
||
519 | * Performs a Model creation commit and persists to the database. |
||
520 | * |
||
521 | * @param Model $model |
||
522 | * @return Model |
||
523 | */ |
||
524 | View Code Duplication | private function doCommitCreate(Model $model) |
|
536 | |||
537 | /** |
||
538 | * Performs a Model delete commit and persists to the database. |
||
539 | * |
||
540 | * @param Model $model |
||
541 | * @return Model |
||
542 | */ |
||
543 | View Code Duplication | private function doCommitDelete(Model $model) |
|
553 | |||
554 | /** |
||
555 | * Performs a Model update commit and persists to the database. |
||
556 | * |
||
557 | * @param Model $model |
||
558 | * @return Model |
||
559 | */ |
||
560 | View Code Duplication | private function doCommitUpdate(Model $model) |
|
571 | |||
572 | /** |
||
573 | * Validates that an embed name/type can be set to an owning embed metadata type. |
||
574 | * |
||
575 | * @param EmbedMetadata $owningMeta The metadata the type will be added to. |
||
576 | * @param string $nameToCheck The name to check. |
||
577 | * @return self |
||
578 | * @throws StoreException If the type to add is not supported. |
||
579 | */ |
||
580 | public function validateEmbedSet(EmbedMetadata $owningMeta, $nameToCheck) |
||
587 | |||
588 | /** |
||
589 | * Validates that a model type can be set to an owning metadata type. |
||
590 | * |
||
591 | * @param EntityMetadata $owningMeta The metadata the type will be added to. |
||
592 | * @param string $typeToAdd The type to add. |
||
593 | * @return self |
||
594 | * @throws StoreException If the type to add is not supported. |
||
595 | */ |
||
596 | public function validateRelationshipSet(EntityMetadata $owningMeta, $typeToAdd) |
||
608 | |||
609 | /** |
||
610 | * Converts an attribute value to the proper Modlr data type. |
||
611 | * |
||
612 | * @param string $dataType The data type, such as string, integer, boolean, etc. |
||
613 | * @param mixed $value The value to convert. |
||
614 | * @return mixed |
||
615 | */ |
||
616 | public function convertAttributeValue($dataType, $value) |
||
620 | |||
621 | /** |
||
622 | * Determines if a model is eligible for commit. |
||
623 | * |
||
624 | * @todo Does delete need to be here? |
||
625 | * @param Model $model |
||
626 | * @return bool |
||
627 | */ |
||
628 | protected function shouldCommit(Model $model) |
||
633 | |||
634 | /** |
||
635 | * Generates a new identifier value for a model type. |
||
636 | * |
||
637 | * @param string $typeKey The model type. |
||
638 | * @return string |
||
639 | */ |
||
640 | protected function generateIdentifier($typeKey) |
||
644 | |||
645 | /** |
||
646 | * Converts the id value to a normalized string. |
||
647 | * |
||
648 | * @param mixed $identenfier The identifier to convert. |
||
649 | * @return string |
||
650 | */ |
||
651 | protected function convertId($identifier) |
||
655 | |||
656 | /** |
||
657 | * Gets the metadata for a model type. |
||
658 | * |
||
659 | * @param string $typeKey The model type. |
||
660 | * @return EntityMetadata |
||
661 | */ |
||
662 | public function getMetadataForType($typeKey) |
||
666 | |||
667 | /** |
||
668 | * Gets the metadata for a relationship. |
||
669 | * |
||
670 | * @param RelationshipMetadata $relMeta The relationship metadata. |
||
671 | * @return EntityMetadata |
||
672 | */ |
||
673 | public function getMetadataForRelationship(RelationshipMetadata $relMeta) |
||
677 | |||
678 | /** |
||
679 | * Determines if an array is sequential. |
||
680 | * |
||
681 | * @param array $arr |
||
682 | * @return bool |
||
683 | */ |
||
684 | protected function isSequentialArray(array $arr) |
||
691 | } |
||
692 |
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.