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 | * The raw result hydrator. |
||
44 | * |
||
45 | * @var Hydrator |
||
46 | */ |
||
47 | private $hydrator; |
||
48 | |||
49 | /** |
||
50 | * @var StorageMetadataFactory |
||
51 | */ |
||
52 | private $smf; |
||
53 | |||
54 | /** |
||
55 | * Constructor. |
||
56 | * |
||
57 | * @param Connection $connection |
||
58 | * @param StorageMetadataFactory $smf |
||
59 | */ |
||
60 | public function __construct(Connection $connection, StorageMetadataFactory $smf) |
||
68 | |||
69 | /** |
||
70 | * {@inheritDoc} |
||
71 | */ |
||
72 | public function getPersisterKey() |
||
76 | |||
77 | /** |
||
78 | * {@inheritDoc} |
||
79 | */ |
||
80 | public function getPersistenceMetadataFactory() |
||
84 | |||
85 | /** |
||
86 | * {@inheritDoc} |
||
87 | * @todo Implement sorting and pagination (limit/skip). |
||
88 | */ |
||
89 | public function all(EntityMetadata $metadata, Store $store, array $identifiers = []) |
||
95 | |||
96 | /** |
||
97 | * {@inheritDoc} |
||
98 | */ |
||
99 | public function query(EntityMetadata $metadata, Store $store, array $criteria, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
104 | |||
105 | /** |
||
106 | * {@inheritDoc} |
||
107 | */ |
||
108 | public function inverse(EntityMetadata $owner, EntityMetadata $rel, Store $store, array $identifiers, $inverseField) |
||
114 | |||
115 | /** |
||
116 | * {@inheritDoc} |
||
117 | */ |
||
118 | public function retrieve(EntityMetadata $metadata, $identifier, Store $store) |
||
127 | |||
128 | /** |
||
129 | * {@inheritDoc} |
||
130 | * @todo Optimize the changeset to query generation. |
||
131 | */ |
||
132 | public function create(Model $model) |
||
170 | |||
171 | /** |
||
172 | * {@inheritDoc} |
||
173 | * @todo Optimize the changeset to query generation. |
||
174 | */ |
||
175 | public function update(Model $model) |
||
229 | |||
230 | /** |
||
231 | * {@inheritDoc} |
||
232 | */ |
||
233 | public function delete(Model $model) |
||
246 | |||
247 | /** |
||
248 | * {@inheritDoc} |
||
249 | */ |
||
250 | public function generateId($strategy = null) |
||
257 | |||
258 | /** |
||
259 | * @return Formatter |
||
260 | */ |
||
261 | public function getFormatter() |
||
265 | |||
266 | /** |
||
267 | * @return Hydrator |
||
268 | */ |
||
269 | public function getHydrator() |
||
273 | |||
274 | /** |
||
275 | * {@inheritDoc} |
||
276 | */ |
||
277 | public function convertId($identifier, $strategy = null) |
||
281 | |||
282 | /** |
||
283 | * {@inheritDoc} |
||
284 | */ |
||
285 | public function getIdentifierKey() |
||
289 | |||
290 | /** |
||
291 | * {@inheritDoc} |
||
292 | */ |
||
293 | public function getPolymorphicKey() |
||
297 | |||
298 | /** |
||
299 | * {@inheritDoc} |
||
300 | */ |
||
301 | public function extractType(EntityMetadata $metadata, array $data) |
||
305 | |||
306 | /** |
||
307 | * Finds records from the database based on the provided metadata and criteria. |
||
308 | * |
||
309 | * @param EntityMetadata $metadata The model metadata that the database should query against. |
||
310 | * @param Store $store The store. |
||
311 | * @param array $criteria The query criteria. |
||
312 | * @param array $fields Fields to include/exclude. |
||
313 | * @param array $sort The sort criteria. |
||
314 | * @param int $offset The starting offset, aka the number of Models to skip. |
||
315 | * @param int $limit The number of Models to limit. |
||
316 | * @return \Doctrine\MongoDB\Cursor |
||
317 | */ |
||
318 | protected function doQuery(EntityMetadata $metadata, Store $store, array $criteria, array $fields = [], array $sort = [], $offset = 0, $limit = 0) |
||
328 | |||
329 | /** |
||
330 | * Gets standard database retrieval criteria for an inverse relationship. |
||
331 | * |
||
332 | * @param EntityMetadata $metadata The entity to retrieve database records for. |
||
333 | * @param string|array $identifiers The IDs to query. |
||
334 | * @return array |
||
335 | */ |
||
336 | protected function getInverseCriteria(EntityMetadata $owner, EntityMetadata $related, $identifiers, $inverseField) |
||
352 | |||
353 | /** |
||
354 | * Gets standard database retrieval criteria for an entity and the provided identifiers. |
||
355 | * |
||
356 | * @param EntityMetadata $metadata The entity to retrieve database records for. |
||
357 | * @param string|array|null $identifiers The IDs to query. |
||
358 | * @return array |
||
359 | */ |
||
360 | protected function getRetrieveCritiera(EntityMetadata $metadata, $identifiers = null) |
||
377 | |||
378 | /** |
||
379 | * Creates a builder object for querying MongoDB based on the provided metadata. |
||
380 | * |
||
381 | * @param EntityMetadata $metadata |
||
382 | * @return \Doctrine\MongoDB\Query\Builder |
||
383 | */ |
||
384 | protected function createQueryBuilder(EntityMetadata $metadata) |
||
388 | |||
389 | /** |
||
390 | * Gets the MongoDB Collection object for a Model. |
||
391 | * |
||
392 | * @param EntityMetadata $metadata |
||
393 | * @return \Doctrine\MongoDB\Collection |
||
394 | */ |
||
395 | protected function getModelCollection(EntityMetadata $metadata) |
||
399 | } |
||
400 |
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.