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 Persister 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 Persister, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
22 | final class Persister implements PersisterInterface |
||
23 | { |
||
24 | const IDENTIFIER_KEY = '_id'; |
||
25 | const POLYMORPHIC_KEY = '_type'; |
||
26 | const PERSISTER_KEY = 'mongodb'; |
||
27 | |||
28 | /** |
||
29 | * The Doctine MongoDB connection. |
||
30 | * |
||
31 | * @var Connection |
||
32 | */ |
||
33 | private $connection; |
||
34 | |||
35 | /** |
||
36 | * The query/database operations formatter. |
||
37 | * |
||
38 | * @var Formatter |
||
39 | */ |
||
40 | private $formatter; |
||
41 | |||
42 | /** |
||
43 | * @var StorageMetadataFactory |
||
44 | */ |
||
45 | private $smf; |
||
46 | |||
47 | /** |
||
48 | * Constructor. |
||
49 | * |
||
50 | * @param Connection $connection |
||
51 | * @param StorageMetadataFactory $smf |
||
52 | */ |
||
53 | public function __construct(Connection $connection, StorageMetadataFactory $smf) |
||
60 | |||
61 | /** |
||
62 | * {@inheritDoc} |
||
63 | */ |
||
64 | public function getPersisterKey() |
||
68 | |||
69 | /** |
||
70 | * {@inheritDoc} |
||
71 | */ |
||
72 | public function getPersistenceMetadataFactory() |
||
76 | |||
77 | /** |
||
78 | * {@inheritDoc} |
||
79 | * @todo Implement sorting and pagination (limit/skip). |
||
80 | */ |
||
81 | public function all(EntityMetadata $metadata, Store $store, array $identifiers = []) |
||
87 | |||
88 | /** |
||
89 | * {@inheritDoc} |
||
90 | */ |
||
91 | public function query(EntityMetadata $metadata, Store $store, array $criteria, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
96 | |||
97 | /** |
||
98 | * {@inheritDoc} |
||
99 | */ |
||
100 | public function inverse(EntityMetadata $owner, EntityMetadata $rel, Store $store, array $identifiers, $inverseField) |
||
106 | |||
107 | /** |
||
108 | * {@inheritDoc} |
||
109 | */ |
||
110 | public function retrieve(EntityMetadata $metadata, $identifier, Store $store) |
||
119 | |||
120 | /** |
||
121 | * {@inheritDoc} |
||
122 | * @todo Optimize the changeset to query generation. |
||
123 | */ |
||
124 | public function create(Model $model) |
||
162 | |||
163 | /** |
||
164 | * Prepares and formats an attribute value for proper insertion into the database. |
||
165 | * |
||
166 | * @deprecated |
||
167 | * @param AttributeMetadata $attrMeta |
||
168 | * @param mixed $value |
||
169 | * @return mixed |
||
170 | */ |
||
171 | View Code Duplication | protected function prepareAttribute(AttributeMetadata $attrMeta, $value) |
|
179 | |||
180 | /** |
||
181 | * Prepares and formats a has-one relationship model for proper insertion into the database. |
||
182 | * |
||
183 | * @deprecated |
||
184 | * @param RelationshipMetadata $relMeta |
||
185 | * @param Model|null $model |
||
186 | * @return mixed |
||
187 | */ |
||
188 | View Code Duplication | protected function prepareHasOne(RelationshipMetadata $relMeta, Model $model = null) |
|
195 | |||
196 | /** |
||
197 | * Prepares and formats a has-many relationship model set for proper insertion into the database. |
||
198 | * |
||
199 | * @deprecated |
||
200 | * @param RelationshipMetadata $relMeta |
||
201 | * @param Model[]|null $models |
||
202 | * @return mixed |
||
203 | */ |
||
204 | View Code Duplication | protected function prepareHasMany(RelationshipMetadata $relMeta, array $models = null) |
|
215 | |||
216 | /** |
||
217 | * Creates a reference for storage of a related model in the database |
||
218 | * |
||
219 | * @deprecated |
||
220 | * @param RelationshipMetadata $relMeta |
||
221 | * @param Model $model |
||
222 | * @return mixed |
||
223 | */ |
||
224 | protected function createReference(RelationshipMetadata $relMeta, Model $model) |
||
233 | |||
234 | /** |
||
235 | * {@inheritDoc} |
||
236 | * @todo Optimize the changeset to query generation. |
||
237 | */ |
||
238 | public function update(Model $model) |
||
292 | |||
293 | /** |
||
294 | * {@inheritDoc} |
||
295 | */ |
||
296 | public function delete(Model $model) |
||
309 | |||
310 | /** |
||
311 | * {@inheritDoc} |
||
312 | */ |
||
313 | public function generateId($strategy = null) |
||
320 | |||
321 | /** |
||
322 | * @return Formatter |
||
323 | */ |
||
324 | public function getFormatter() |
||
328 | |||
329 | /** |
||
330 | * {@inheritDoc} |
||
331 | */ |
||
332 | public function convertId($identifier, $strategy = null) |
||
336 | |||
337 | /** |
||
338 | * {@inheritDoc} |
||
339 | */ |
||
340 | public function getIdentifierKey() |
||
344 | |||
345 | /** |
||
346 | * {@inheritDoc} |
||
347 | */ |
||
348 | public function getPolymorphicKey() |
||
352 | |||
353 | /** |
||
354 | * {@inheritDoc} |
||
355 | */ |
||
356 | public function extractType(EntityMetadata $metadata, array $data) |
||
366 | |||
367 | /** |
||
368 | * Finds records from the database based on the provided metadata and criteria. |
||
369 | * |
||
370 | * @param EntityMetadata $metadata The model metadata that the database should query against. |
||
371 | * @param Store $store The store. |
||
372 | * @param array $criteria The query criteria. |
||
373 | * @param array $fields Fields to include/exclude. |
||
374 | * @param array $sort The sort criteria. |
||
375 | * @param int $offset The starting offset, aka the number of Models to skip. |
||
376 | * @param int $limit The number of Models to limit. |
||
377 | * @return \Doctrine\MongoDB\Cursor |
||
378 | */ |
||
379 | protected function doQuery(EntityMetadata $metadata, Store $store, array $criteria, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
389 | |||
390 | /** |
||
391 | * Processes multiple, raw MongoDB results an converts them into an array of standardized Record objects. |
||
392 | * |
||
393 | * @param EntityMetadata $metadata |
||
394 | * @param array $results |
||
395 | * @param Store $store |
||
396 | * @return Record[] |
||
397 | */ |
||
398 | protected function hydrateRecords(EntityMetadata $metadata, array $results, Store $store) |
||
406 | |||
407 | /** |
||
408 | * Processes raw MongoDB data an converts it into a standardized Record object. |
||
409 | * |
||
410 | * @param EntityMetadata $metadata |
||
411 | * @param array $data |
||
412 | * @param Store $store |
||
413 | * @return Record |
||
414 | */ |
||
415 | protected function hydrateRecord(EntityMetadata $metadata, array $data, Store $store) |
||
441 | |||
442 | /** |
||
443 | * Extracts a standard relationship array that the store expects from a raw MongoDB reference value. |
||
444 | * |
||
445 | * @param RelationshipMetadata $relMeta |
||
446 | * @param mixed $reference |
||
447 | * @return array |
||
448 | * @throws \RuntimeException If the relationship could not be extracted. |
||
449 | */ |
||
450 | protected function extractRelationship(RelationshipMetadata $relMeta, $reference) |
||
474 | |||
475 | /** |
||
476 | * Gets standard database retrieval criteria for an inverse relationship. |
||
477 | * |
||
478 | * @param EntityMetadata $metadata The entity to retrieve database records for. |
||
479 | * @param string|array $identifiers The IDs to query. |
||
480 | * @return array |
||
481 | */ |
||
482 | protected function getInverseCriteria(EntityMetadata $owner, EntityMetadata $related, $identifiers, $inverseField) |
||
498 | |||
499 | /** |
||
500 | * Gets standard database retrieval criteria for an entity and the provided identifiers. |
||
501 | * |
||
502 | * @param EntityMetadata $metadata The entity to retrieve database records for. |
||
503 | * @param string|array|null $identifiers The IDs to query. |
||
504 | * @return array |
||
505 | */ |
||
506 | protected function getRetrieveCritiera(EntityMetadata $metadata, $identifiers = null) |
||
523 | |||
524 | /** |
||
525 | * Creates a builder object for querying MongoDB based on the provided metadata. |
||
526 | * |
||
527 | * @param EntityMetadata $metadata |
||
528 | * @return \Doctrine\MongoDB\Query\Builder |
||
529 | */ |
||
530 | protected function createQueryBuilder(EntityMetadata $metadata) |
||
534 | |||
535 | /** |
||
536 | * Gets the MongoDB Collection object for a Model. |
||
537 | * |
||
538 | * @param EntityMetadata $metadata |
||
539 | * @return \Doctrine\MongoDB\Collection |
||
540 | */ |
||
541 | protected function getModelCollection(EntityMetadata $metadata) |
||
545 | |||
546 | /** |
||
547 | * Determines if the current id strategy is supported. |
||
548 | * |
||
549 | * @deprecated |
||
550 | * @param string|null $strategy |
||
551 | * @return bool |
||
552 | */ |
||
553 | protected function isIdStrategySupported($strategy) |
||
557 | } |
||
558 |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_function
expects aPost
object, and outputs the author of the post. The base classPost
returns a simple string and outputting a simple string will work just fine. However, the child classBlogPost
which is a sub-type ofPost
instead decided to return anobject
, and is therefore violating the SOLID principles. If aBlogPost
were passed tomy_function
, PHP would not complain, but ultimately fail when executing thestrtoupper
call in its body.