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 EEM_Datetime 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 EEM_Datetime, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class EEM_Datetime extends EEM_Soft_Delete_Base |
||
14 | { |
||
15 | |||
16 | /** |
||
17 | * @var EEM_Datetime $_instance |
||
18 | */ |
||
19 | protected static $_instance; |
||
20 | |||
21 | |||
22 | /** |
||
23 | * private constructor to prevent direct creation |
||
24 | * |
||
25 | * @param string $timezone A string representing the timezone we want to set for returned Date Time Strings |
||
26 | * (and any incoming timezone data that gets saved). |
||
27 | * Note this just sends the timezone info to the date time model field objects. |
||
28 | * Default is NULL |
||
29 | * (and will be assumed using the set timezone in the 'timezone_string' wp option) |
||
30 | * @throws EE_Error |
||
31 | * @throws InvalidArgumentException |
||
32 | * @throws InvalidArgumentException |
||
33 | */ |
||
34 | protected function __construct($timezone) |
||
153 | |||
154 | |||
155 | /** |
||
156 | * create new blank datetime |
||
157 | * |
||
158 | * @access public |
||
159 | * @return EE_Datetime[] array on success, FALSE on fail |
||
160 | * @throws EE_Error |
||
161 | * @throws InvalidArgumentException |
||
162 | * @throws InvalidDataTypeException |
||
163 | * @throws ReflectionException |
||
164 | * @throws InvalidInterfaceException |
||
165 | */ |
||
166 | public function create_new_blank_datetime() |
||
242 | |||
243 | |||
244 | /** |
||
245 | * Validates whether the start_time and end_time are in the expected format. |
||
246 | * |
||
247 | * @param array $start_time |
||
248 | * @param array $end_time |
||
249 | * @throws InvalidArgumentException |
||
250 | * @throws InvalidDataTypeException |
||
251 | */ |
||
252 | private function validateStartAndEndTimeForBlankDate(array $start_time, array $end_time) |
||
279 | |||
280 | |||
281 | /** |
||
282 | * get event start date from db |
||
283 | * |
||
284 | * @access public |
||
285 | * @param int $EVT_ID |
||
286 | * @return EE_Datetime[] array on success, FALSE on fail |
||
287 | * @throws EE_Error |
||
288 | * @throws ReflectionException |
||
289 | */ |
||
290 | public function get_all_event_dates($EVT_ID = 0) |
||
301 | |||
302 | |||
303 | /** |
||
304 | * get all datetimes attached to an event ordered by the DTT_order field |
||
305 | * |
||
306 | * @public |
||
307 | * @param int $EVT_ID event id |
||
308 | * @param boolean $include_expired |
||
309 | * @param boolean $include_deleted |
||
310 | * @param int $limit If included then limit the count of results by |
||
311 | * the given number |
||
312 | * @return EE_Datetime[] |
||
313 | * @throws EE_Error |
||
314 | */ |
||
315 | View Code Duplication | public function get_datetimes_for_event_ordered_by_DTT_order( |
|
328 | |||
329 | |||
330 | /** |
||
331 | * Gets the datetimes for the event (with the given limit), and orders them by "importance". |
||
332 | * By importance, we mean that the primary datetimes are most important (DEPRECATED FOR NOW), |
||
333 | * and then the earlier datetimes are the most important. |
||
334 | * Maybe we'll want this to take into account datetimes that haven't already passed, but we don't yet. |
||
335 | * |
||
336 | * @param int $EVT_ID |
||
337 | * @param int $limit |
||
338 | * @return EE_Datetime[]|EE_Base_Class[] |
||
339 | * @throws EE_Error |
||
340 | */ |
||
341 | public function get_datetimes_for_event_ordered_by_importance(int $EVT_ID, $limit = 0) |
||
348 | |||
349 | |||
350 | /** |
||
351 | * @param int $EVT_ID |
||
352 | * @param boolean $include_expired |
||
353 | * @param boolean $include_deleted |
||
354 | * @return EE_Datetime |
||
355 | * @throws EE_Error |
||
356 | */ |
||
357 | public function get_oldest_datetime_for_event( |
||
373 | |||
374 | |||
375 | /** |
||
376 | * Gets the 'primary' datetime for an event. |
||
377 | * |
||
378 | * @param int $EVT_ID |
||
379 | * @param bool $try_to_exclude_expired |
||
380 | * @param bool $try_to_exclude_deleted |
||
381 | * @return EE_Datetime |
||
382 | * @throws EE_Error |
||
383 | */ |
||
384 | public function get_primary_datetime_for_event( |
||
403 | |||
404 | |||
405 | /** |
||
406 | * Gets ALL the datetimes for an event (including trashed ones, for now), ordered |
||
407 | * only by start date |
||
408 | * |
||
409 | * @param int $EVT_ID |
||
410 | * @param boolean $include_expired |
||
411 | * @param boolean $include_deleted |
||
412 | * @param int $limit |
||
413 | * @return EE_Datetime[] |
||
414 | * @throws EE_Error |
||
415 | */ |
||
416 | View Code Duplication | public function get_datetimes_for_event_ordered_by_start_time( |
|
432 | |||
433 | |||
434 | /** |
||
435 | * Gets ALL the datetimes for an ticket (including trashed ones, for now), ordered |
||
436 | * only by start date |
||
437 | * |
||
438 | * @param int $TKT_ID |
||
439 | * @param boolean $include_expired |
||
440 | * @param boolean $include_deleted |
||
441 | * @param int $limit |
||
442 | * @return EE_Datetime[] |
||
443 | * @throws EE_Error |
||
444 | */ |
||
445 | View Code Duplication | public function get_datetimes_for_ticket_ordered_by_start_time( |
|
457 | |||
458 | |||
459 | /** |
||
460 | * Gets all the datetimes for a ticket (including trashed ones, for now), ordered by the DTT_order for the |
||
461 | * datetimes. |
||
462 | * |
||
463 | * @param int $TKT_ID ID of ticket to retrieve the datetimes for |
||
464 | * @param boolean $include_expired whether to include expired datetimes or not |
||
465 | * @param boolean $include_deleted whether to include trashed datetimes or not. |
||
466 | * @param int|null $limit if null, no limit, if int then limit results by |
||
467 | * that number |
||
468 | * @return EE_Datetime[] |
||
469 | * @throws EE_Error |
||
470 | */ |
||
471 | View Code Duplication | public function get_datetimes_for_ticket_ordered_by_DTT_order( |
|
483 | |||
484 | |||
485 | /** |
||
486 | * Gets the most important datetime for a particular event (ie, the primary event usually. But if for some WACK |
||
487 | * reason it doesn't exist, we consider the earliest event the most important) |
||
488 | * |
||
489 | * @param int $EVT_ID |
||
490 | * @return EE_Datetime |
||
491 | * @throws EE_Error |
||
492 | */ |
||
493 | public function get_most_important_datetime_for_event(int $EVT_ID) |
||
501 | |||
502 | |||
503 | /** |
||
504 | * This returns a wpdb->results Array of all DTT month and years matching the incoming query params and |
||
505 | * grouped by month and year. |
||
506 | * |
||
507 | * @param array $where_params @see |
||
508 | * https://github.com/eventespresso/event-espresso-core/tree/master/docs/G--Model-System/model-query-params.md#0-where-conditions |
||
509 | * @param string $evt_active_status A string representing the evt active status to filter the months by. |
||
510 | * Can be: |
||
511 | * - '' = no filter |
||
512 | * - upcoming = Published events with at least one upcoming datetime. |
||
513 | * - expired = Events with all datetimes expired. |
||
514 | * - active = Events that are published and have at least one datetime that |
||
515 | * starts before now and ends after now. |
||
516 | * - inactive = Events that are either not published. |
||
517 | * @return EE_Base_Class[] |
||
518 | * @throws EE_Error |
||
519 | * @throws InvalidArgumentException |
||
520 | * @throws InvalidArgumentException |
||
521 | */ |
||
522 | public function get_dtt_months_and_years(array $where_params, $evt_active_status = '') |
||
601 | |||
602 | |||
603 | /** |
||
604 | * Updates the DTT_sold attribute on each datetime (based on the registrations |
||
605 | * for the tickets for each datetime) |
||
606 | * |
||
607 | * @param EE_Base_Class[]|EE_Datetime[] $datetimes |
||
608 | * @throws EE_Error |
||
609 | * @throws ReflectionException |
||
610 | */ |
||
611 | public function update_sold(array $datetimes) |
||
625 | |||
626 | |||
627 | /** |
||
628 | * Gets the total number of tickets available at a particular datetime |
||
629 | * (does NOT take into account the datetime's spaces available) |
||
630 | * |
||
631 | * @param int $DTT_ID |
||
632 | * @param array $query_params |
||
633 | * @return int of tickets available. If sold out, return less than 1. If infinite, returns EE_INF, IF there are NO |
||
634 | * tickets attached to datetime then FALSE is returned. |
||
635 | * @throws EE_Error |
||
636 | * @throws ReflectionException |
||
637 | */ |
||
638 | public function sum_tickets_currently_available_at_datetime(int $DTT_ID, array $query_params = []) |
||
646 | |||
647 | |||
648 | /** |
||
649 | * This returns an array of counts of datetimes in the database for each Datetime status that can be queried. |
||
650 | * |
||
651 | * @param array $stati_to_include If included you can restrict the statuses we return counts for by including the |
||
652 | * stati you want counts for as values in the array. An empty array returns counts |
||
653 | * for all valid stati. |
||
654 | * @param array $query_params If included can be used to refine the conditions for returning the count (i.e. |
||
655 | * only for Datetimes connected to a specific event, or specific ticket. |
||
656 | * @return array The value returned is an array indexed by Datetime Status and the values are the counts. The |
||
657 | * @throws EE_Error |
||
658 | * stati used as index keys are: EE_Datetime::active EE_Datetime::upcoming |
||
659 | * EE_Datetime::expired |
||
660 | */ |
||
661 | public function get_datetime_counts_by_status(array $stati_to_include = [], array $query_params = []) |
||
697 | |||
698 | |||
699 | /** |
||
700 | * Returns the specific count for a given Datetime status matching any given query_params. |
||
701 | * |
||
702 | * @param string $status Valid string representation for Datetime status requested. (Defaults to Active). |
||
703 | * @param array $query_params |
||
704 | * @return int |
||
705 | * @throws EE_Error |
||
706 | */ |
||
707 | public function get_datetime_count_for_status($status = EE_Datetime::active, array $query_params = []) |
||
712 | |||
713 | |||
714 | /** |
||
715 | * @return bool|int |
||
716 | * @since $VID:$ |
||
717 | */ |
||
718 | private function prepModelForQuery() |
||
724 | |||
725 | |||
726 | /** |
||
727 | * @param array $query_params |
||
728 | * @param bool|int $prev_data_prep_value |
||
729 | * @return EE_Base_Class[]|EE_Datetime[] |
||
730 | * @throws EE_Error |
||
731 | * @since $VID:$ |
||
732 | */ |
||
733 | private function getDatetimesAndRestoreModel(array $query_params, $prev_data_prep_value) |
||
739 | |||
740 | |||
741 | /** |
||
742 | * @param array $query_params |
||
743 | * @param int $limit |
||
744 | * @param string $order_by |
||
745 | * @param string $order |
||
746 | * @return array |
||
747 | * @since $VID:$ |
||
748 | */ |
||
749 | private function addDefaultQueryParams(array $query_params, $limit = 0, $order_by = 'DTT_EVT_start', $order = 'ASC') |
||
755 | |||
756 | |||
757 | /** |
||
758 | * @param array $query_params |
||
759 | * @param string $default_where_conditions |
||
760 | * @return array |
||
761 | * @since $VID:$ |
||
762 | */ |
||
763 | private function addDefaultWhereConditions( |
||
770 | |||
771 | |||
772 | /** |
||
773 | * @param array $where_params |
||
774 | * @param bool $include_deleted |
||
775 | * @param bool $include_expired |
||
776 | * @return array |
||
777 | * @since $VID:$ |
||
778 | */ |
||
779 | private function addDefaultWhereParams(array $where_params, bool $include_deleted = true, bool $include_expired = true) |
||
785 | |||
786 | |||
787 | /** |
||
788 | * @param array $where_params |
||
789 | * @param bool $include_deleted |
||
790 | * @return array |
||
791 | * @since $VID:$ |
||
792 | */ |
||
793 | private function addDeletedWhereParams(array $where_params, bool $include_deleted = true) |
||
799 | |||
800 | |||
801 | /** |
||
802 | * @param array $where_params |
||
803 | * @param bool $include_expired |
||
804 | * @return array |
||
805 | * @since $VID:$ |
||
806 | */ |
||
807 | private function addExpiredWhereParams(array $where_params, bool $include_expired = true) |
||
814 | |||
815 | |||
816 | /** |
||
817 | * @param array $query_params |
||
818 | * @param int $limit |
||
819 | * @return array |
||
820 | * @since $VID:$ |
||
821 | */ |
||
822 | private function addLimitQueryParams(array $query_params, $limit = 0) |
||
829 | |||
830 | |||
831 | /** |
||
832 | * @param array $query_params |
||
833 | * @param string $order_by |
||
834 | * @param string $order |
||
835 | * @return array |
||
836 | * @since $VID:$ |
||
837 | */ |
||
838 | private function addOrderByQueryParams(array $query_params, $order_by = 'DTT_EVT_start', $order = 'ASC') |
||
846 | } |
||
847 |
Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.
Let’s take a look at an example:
As you can see in this example, the array
$myArray
is initialized the first time when the foreach loop is entered. You can also see that the value of thebar
key is only written conditionally; thus, its value might result from a previous iteration.This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.