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 MetadataTable 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 MetadataTable, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
21 | class MetadataTable { |
||
22 | /** @var array */ |
||
23 | private $independents = array(); |
||
24 | |||
25 | /** @var Cache */ |
||
26 | private $cache; |
||
27 | |||
28 | /** @var Database */ |
||
29 | private $db; |
||
30 | |||
31 | /** @var EntityTable */ |
||
32 | private $entityTable; |
||
33 | |||
34 | /** @var MetastringsTable */ |
||
35 | private $metastringsTable; |
||
36 | |||
37 | /** @var Events */ |
||
38 | private $events; |
||
39 | |||
40 | /** @var Session */ |
||
41 | private $session; |
||
42 | |||
43 | /** @var string */ |
||
44 | private $table; |
||
45 | |||
46 | /** |
||
47 | * Constructor |
||
48 | * |
||
49 | * @param Cache $cache A cache for this table |
||
50 | * @param Database $db The Elgg database |
||
51 | * @param EntityTable $entityTable The entities table |
||
52 | * @param Events $events The events registry |
||
53 | * @param MetastringsTable $metastringsTable The metastrings table |
||
54 | * @param Session $session The session |
||
55 | */ |
||
56 | 1 | public function __construct( |
|
71 | |||
72 | /** |
||
73 | * Get a specific metadata object by its id. |
||
74 | * If you want multiple metadata objects, use |
||
75 | * {@link elgg_get_metadata()}. |
||
76 | * |
||
77 | * @param int $id The id of the metadata object being retrieved. |
||
78 | * |
||
79 | * @return \ElggMetadata|false false if not found |
||
80 | */ |
||
81 | function get($id) { |
||
84 | |||
85 | /** |
||
86 | * Deletes metadata using its ID. |
||
87 | * |
||
88 | * @param int $id The metadata ID to delete. |
||
89 | * @return bool |
||
90 | */ |
||
91 | function delete($id) { |
||
96 | |||
97 | /** |
||
98 | * Create a new metadata object, or update an existing one. |
||
99 | * |
||
100 | * Metadata can be an array by setting allow_multiple to true, but it is an |
||
101 | * indexed array with no control over the indexing. |
||
102 | * |
||
103 | * @param int $entity_guid The entity to attach the metadata to |
||
104 | * @param string $name Name of the metadata |
||
105 | * @param string $value Value of the metadata |
||
106 | * @param string $value_type 'text', 'integer', or '' for automatic detection |
||
107 | * @param int $owner_guid GUID of entity that owns the metadata. Default is logged in user. |
||
108 | * @param int $access_id Default is ACCESS_PRIVATE |
||
109 | * @param bool $allow_multiple Allow multiple values for one key. Default is false |
||
110 | * |
||
111 | * @return int|false id of metadata or false if failure |
||
112 | */ |
||
113 | function create($entity_guid, $name, $value, $value_type = '', $owner_guid = 0, |
||
183 | |||
184 | /** |
||
185 | * Update a specific piece of metadata. |
||
186 | * |
||
187 | * @param int $id ID of the metadata to update |
||
188 | * @param string $name Metadata name |
||
189 | * @param string $value Metadata value |
||
190 | * @param string $value_type Value type |
||
191 | * @param int $owner_guid Owner guid |
||
192 | * @param int $access_id Access ID |
||
193 | * |
||
194 | * @return bool |
||
195 | */ |
||
196 | function update($id, $name, $value, $value_type, $owner_guid, $access_id) { |
||
260 | |||
261 | /** |
||
262 | * This function creates metadata from an associative array of "key => value" pairs. |
||
263 | * |
||
264 | * To achieve an array for a single key, pass in the same key multiple times with |
||
265 | * allow_multiple set to true. This creates an indexed array. It does not support |
||
266 | * associative arrays and there is no guarantee on the ordering in the array. |
||
267 | * |
||
268 | * @param int $entity_guid The entity to attach the metadata to |
||
269 | * @param array $name_and_values Associative array - a value can be a string, number, bool |
||
270 | * @param string $value_type 'text', 'integer', or '' for automatic detection |
||
271 | * @param int $owner_guid GUID of entity that owns the metadata |
||
272 | * @param int $access_id Default is ACCESS_PRIVATE |
||
273 | * @param bool $allow_multiple Allow multiple values for one key. Default is false |
||
274 | * |
||
275 | * @return bool |
||
276 | */ |
||
277 | function createFromArray($entity_guid, array $name_and_values, $value_type, $owner_guid, |
||
289 | |||
290 | /** |
||
291 | * Returns metadata. Accepts all elgg_get_entities() options for entity |
||
292 | * restraints. |
||
293 | * |
||
294 | * @see elgg_get_entities |
||
295 | * |
||
296 | * @warning 1.7's find_metadata() didn't support limits and returned all metadata. |
||
297 | * This function defaults to a limit of 25. There is probably not a reason |
||
298 | * for you to return all metadata unless you're exporting an entity, |
||
299 | * have other restraints in place, or are doing something horribly |
||
300 | * wrong in your code. |
||
301 | * |
||
302 | * @param array $options Array in format: |
||
303 | * |
||
304 | * metadata_names => null|ARR metadata names |
||
305 | * metadata_values => null|ARR metadata values |
||
306 | * metadata_ids => null|ARR metadata ids |
||
307 | * metadata_case_sensitive => BOOL Overall Case sensitive |
||
308 | * metadata_owner_guids => null|ARR guids for metadata owners |
||
309 | * metadata_created_time_lower => INT Lower limit for created time. |
||
310 | * metadata_created_time_upper => INT Upper limit for created time. |
||
311 | * metadata_calculation => STR Perform the MySQL function on the metadata values returned. |
||
312 | * The "metadata_calculation" option causes this function to |
||
313 | * return the result of performing a mathematical calculation on |
||
314 | * all metadata that match the query instead of returning |
||
315 | * \ElggMetadata objects. |
||
316 | * |
||
317 | * @return \ElggMetadata[]|mixed |
||
318 | */ |
||
319 | View Code Duplication | function getAll(array $options = array()) { |
|
331 | |||
332 | /** |
||
333 | * Deletes metadata based on $options. |
||
334 | * |
||
335 | * @warning Unlike elgg_get_metadata() this will not accept an empty options array! |
||
336 | * This requires at least one constraint: metadata_owner_guid(s), |
||
337 | * metadata_name(s), metadata_value(s), or guid(s) must be set. |
||
338 | * |
||
339 | * @param array $options An options array. {@link elgg_get_metadata()} |
||
340 | * @return bool|null true on success, false on failure, null if no metadata to delete. |
||
341 | */ |
||
342 | View Code Duplication | function deleteAll(array $options) { |
|
355 | |||
356 | /** |
||
357 | * Disables metadata based on $options. |
||
358 | * |
||
359 | * @warning Unlike elgg_get_metadata() this will not accept an empty options array! |
||
360 | * |
||
361 | * @param array $options An options array. {@link elgg_get_metadata()} |
||
362 | * @return bool|null true on success, false on failure, null if no metadata disabled. |
||
363 | */ |
||
364 | View Code Duplication | function disableAll(array $options) { |
|
378 | |||
379 | /** |
||
380 | * Enables metadata based on $options. |
||
381 | * |
||
382 | * @warning Unlike elgg_get_metadata() this will not accept an empty options array! |
||
383 | * |
||
384 | * @warning In order to enable metadata, you must first use |
||
385 | * {@link access_show_hidden_entities()}. |
||
386 | * |
||
387 | * @param array $options An options array. {@link elgg_get_metadata()} |
||
388 | * @return bool|null true on success, false on failure, null if no metadata enabled. |
||
389 | */ |
||
390 | View Code Duplication | function enableAll(array $options) { |
|
400 | |||
401 | /** |
||
402 | * Returns entities based upon metadata. Also accepts all |
||
403 | * options available to elgg_get_entities(). Supports |
||
404 | * the singular option shortcut. |
||
405 | * |
||
406 | * @note Using metadata_names and metadata_values results in a |
||
407 | * "names IN (...) AND values IN (...)" clause. This is subtly |
||
408 | * differently than default multiple metadata_name_value_pairs, which use |
||
409 | * "(name = value) AND (name = value)" clauses. |
||
410 | * |
||
411 | * When in doubt, use name_value_pairs. |
||
412 | * |
||
413 | * To ask for entities that do not have a metadata value, use a custom |
||
414 | * where clause like this: |
||
415 | * |
||
416 | * $options['wheres'][] = "NOT EXISTS ( |
||
417 | * SELECT 1 FROM {$dbprefix}metadata md |
||
418 | * WHERE md.entity_guid = e.guid |
||
419 | * AND md.name_id = $name_metastring_id |
||
420 | * AND md.value_id = $value_metastring_id)"; |
||
421 | * |
||
422 | * Note the metadata name and value has been denormalized in the above example. |
||
423 | * |
||
424 | * @see elgg_get_entities |
||
425 | * |
||
426 | * @param array $options Array in format: |
||
427 | * |
||
428 | * metadata_names => null|ARR metadata names |
||
429 | * |
||
430 | * metadata_values => null|ARR metadata values |
||
431 | * |
||
432 | * metadata_name_value_pairs => null|ARR ( |
||
433 | * name => 'name', |
||
434 | * value => 'value', |
||
435 | * 'operand' => '=', |
||
436 | * 'case_sensitive' => true |
||
437 | * ) |
||
438 | * Currently if multiple values are sent via |
||
439 | * an array (value => array('value1', 'value2') |
||
440 | * the pair's operand will be forced to "IN". |
||
441 | * If passing "IN" as the operand and a string as the value, |
||
442 | * the value must be a properly quoted and escaped string. |
||
443 | * |
||
444 | * metadata_name_value_pairs_operator => null|STR The operator to use for combining |
||
445 | * (name = value) OPERATOR (name = value); default AND |
||
446 | * |
||
447 | * metadata_case_sensitive => BOOL Overall Case sensitive |
||
448 | * |
||
449 | * order_by_metadata => null|ARR array( |
||
450 | * 'name' => 'metadata_text1', |
||
451 | * 'direction' => ASC|DESC, |
||
452 | * 'as' => text|integer |
||
453 | * ) |
||
454 | * Also supports array('name' => 'metadata_text1') |
||
455 | * |
||
456 | * metadata_owner_guids => null|ARR guids for metadata owners |
||
457 | * |
||
458 | * @return \ElggEntity[]|mixed If count, int. If not count, array. false on errors. |
||
459 | */ |
||
460 | function getEntities(array $options = array()) { |
||
486 | |||
487 | /** |
||
488 | * Returns metadata name and value SQL where for entities. |
||
489 | * NB: $names and $values are not paired. Use $pairs for this. |
||
490 | * Pairs default to '=' operand. |
||
491 | * |
||
492 | * This function is reused for annotations because the tables are |
||
493 | * exactly the same. |
||
494 | * |
||
495 | * @param string $e_table Entities table name |
||
496 | * @param string $n_table Normalized metastrings table name (Where entities, |
||
497 | * values, and names are joined. annotations / metadata) |
||
498 | * @param array|null $names Array of names |
||
499 | * @param array|null $values Array of values |
||
500 | * @param array|null $pairs Array of names / values / operands |
||
501 | * @param string $pair_operator ("AND" or "OR") Operator to use to join the where clauses for pairs |
||
502 | * @param bool $case_sensitive Case sensitive metadata names? |
||
503 | * @param array|null $order_by_metadata Array of names / direction |
||
504 | * @param array|null $owner_guids Array of owner GUIDs |
||
505 | * |
||
506 | * @return false|array False on fail, array('joins', 'wheres') |
||
507 | * @access private |
||
508 | */ |
||
509 | function getEntityMetadataWhereSql($e_table, $n_table, $names = null, $values = null, |
||
762 | |||
763 | /** |
||
764 | * Get the URL for this metadata |
||
765 | * |
||
766 | * By default this links to the export handler in the current view. |
||
767 | * |
||
768 | * @param int $id Metadata ID |
||
769 | * |
||
770 | * @return mixed |
||
771 | */ |
||
772 | function getUrl($id) { |
||
777 | |||
778 | /** |
||
779 | * Mark entities with a particular type and subtype as having access permissions |
||
780 | * that can be changed independently from their parent entity |
||
781 | * |
||
782 | * @param string $type The type - object, user, etc |
||
783 | * @param string $subtype The subtype; all subtypes by default |
||
784 | * |
||
785 | * @return void |
||
786 | */ |
||
787 | function registerMetadataAsIndependent($type, $subtype = '*') { |
||
794 | |||
795 | /** |
||
796 | * Determines whether entities of a given type and subtype should not change |
||
797 | * their metadata in line with their parent entity |
||
798 | * |
||
799 | * @param string $type The type - object, user, etc |
||
800 | * @param string $subtype The entity subtype |
||
801 | * |
||
802 | * @return bool |
||
803 | */ |
||
804 | function isMetadataIndependent($type, $subtype) { |
||
812 | |||
813 | /** |
||
814 | * When an entity is updated, resets the access ID on all of its child metadata |
||
815 | * |
||
816 | * @param string $event The name of the event |
||
817 | * @param string $object_type The type of object |
||
818 | * @param \ElggEntity $object The entity itself |
||
819 | * |
||
820 | * @return true |
||
821 | * @access private Set as private in 1.9.0 |
||
822 | */ |
||
823 | function handleUpdate($event, $object_type, $object) { |
||
834 | |||
835 | } |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)
or! empty(...)
instead.