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 ThreemaGateway_Model_Messages 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 ThreemaGateway_Model_Messages, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 11 | class ThreemaGateway_Model_Messages extends XenForo_Model |
||
| 12 | { |
||
| 13 | /** |
||
| 14 | * @var string database table (prefix) for messages |
||
| 15 | */ |
||
| 16 | const DB_TABLE_MESSAGES = 'xf_threemagw_messages'; |
||
| 17 | |||
| 18 | /** |
||
| 19 | * @var string database table for files |
||
| 20 | */ |
||
| 21 | const DB_TABLE_FILES = 'xf_threemagw_files'; |
||
| 22 | |||
| 23 | /** |
||
| 24 | * @var string database table for acknowledged messages/delivery receipts |
||
| 25 | */ |
||
| 26 | const DB_TABLE_DELIVERY_RECEIPT = 'xf_threemagw_ackmsgs'; |
||
| 27 | |||
| 28 | /** |
||
| 29 | * @var int constant for type code |
||
| 30 | */ |
||
| 31 | const TYPE_DELIVERY_MESSAGE = 0x80; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * @var int constant for type code |
||
| 35 | */ |
||
| 36 | const TYPE_FILE_MESSAGE = 0x17; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * @var int constant for type code |
||
| 40 | */ |
||
| 41 | const TYPE_IMAGE_MESSAGE = 0x02; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * @var int constant for type code |
||
| 45 | */ |
||
| 46 | const TYPE_TEXT_MESSAGE = 0x01; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @var array constant for type code |
||
| 50 | */ |
||
| 51 | const ORDER_CHOICE = [ |
||
| 52 | 'id' => 'message_id', |
||
| 53 | 'date_send' => 'date_send', |
||
| 54 | 'date_received' => 'date_received', |
||
| 55 | 'delivery_state' => 'message.receipt_type', |
||
| 56 | ]; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var array data used when querying |
||
| 60 | */ |
||
| 61 | protected $fetchOptions = [ |
||
| 62 | 'where' => [], |
||
| 63 | 'params' => [] |
||
| 64 | ]; |
||
| 65 | |||
| 66 | /** |
||
| 67 | * Execute this before any query. |
||
| 68 | * |
||
| 69 | * Sets internal values necessary for a correct connection to the database. |
||
| 70 | */ |
||
| 71 | public function preQuery() |
||
| 76 | |||
| 77 | /** |
||
| 78 | * Inject or modify a fetch option manually. |
||
| 79 | * |
||
| 80 | * Sets internal values necessary for a correct connection to the database. |
||
| 81 | * This should best be avoided, when it is not really necessary to change |
||
| 82 | * the value directly. |
||
| 83 | * It can e.g. be used to reset data. When you e.g. want to reset the where |
||
| 84 | * option call it this way: injectFetchOption('where', []). |
||
| 85 | * |
||
| 86 | * @param string $option The option name to inject |
||
| 87 | * @param string $value The value of the option to set. |
||
| 88 | * @param bool $append If set to true, the value is not overriden, but |
||
| 89 | * just appended as an array. (default: false) |
||
| 90 | */ |
||
| 91 | public function injectFetchOption($option, $value, $append = false) |
||
| 99 | |||
| 100 | /** |
||
| 101 | * Rests all fetch options. This is useful to prevent incorrect or |
||
| 102 | * unexpected results when using one model for multiple queries (not |
||
| 103 | * recommend) or for e.g. resetting the options before calling |
||
| 104 | * {@link fetchAll()};. |
||
| 105 | */ |
||
| 106 | public function resetFetchOptions() |
||
| 113 | |||
| 114 | /** |
||
| 115 | * Sets the message ID(s) for the query. |
||
| 116 | * |
||
| 117 | * @param string|array $messageIds one (string) or more (array) message IDs |
||
| 118 | * @param string $tablePrefix The table prefix (optional) |
||
| 119 | */ |
||
| 120 | public function setMessageId($messageIds, $tablePrefix = null) |
||
| 127 | |||
| 128 | /** |
||
| 129 | * Sets the sender Threema ID(s) for querying it/them. |
||
| 130 | * |
||
| 131 | * @param string $threemaIds one (string) or more (array) Threema IDs |
||
| 132 | */ |
||
| 133 | public function setSenderId($threemaIds) |
||
| 140 | |||
| 141 | /** |
||
| 142 | * Sets the type code(s) for querying only one (or a few) type. |
||
| 143 | * |
||
| 144 | * Please use the TYPE_* constants for specifying the type code(s). |
||
| 145 | * You should avoid using this and rather use {@link getMessageDataByType()} |
||
| 146 | * directly if you know the type code. |
||
| 147 | * If you want to limit the types you want to query this method would be a |
||
| 148 | * good way for you to use. |
||
| 149 | * |
||
| 150 | * @param string $typeCodes one (string) or more (array) type codes(s) |
||
| 151 | */ |
||
| 152 | public function setTypeCode($typeCodes) |
||
| 159 | |||
| 160 | /** |
||
| 161 | * Sets a string to look for when querying text messages. |
||
| 162 | * |
||
| 163 | * The string is processed by MySQL via the `LIKE` command and may |
||
| 164 | * therefore contain some wildcards: % for none or any character and |
||
| 165 | * _ for exactly one character. |
||
| 166 | * Attention: This is only possible when using the text message type! |
||
| 167 | * Otherwise your query will fail. |
||
| 168 | * |
||
| 169 | * @param string $keyword a keyword to look for |
||
| 170 | */ |
||
| 171 | public function setKeyword($keyword) |
||
| 176 | |||
| 177 | /** |
||
| 178 | * Sets the a time limit for what messages should be queried. |
||
| 179 | * |
||
| 180 | * @param int|null $dateMin oldest date of messages (optional) |
||
| 181 | * @param int|null $dateMax latest date of messages (optional) |
||
| 182 | * @param string $attribute Set the atttribute to apply this to. |
||
| 183 | */ |
||
| 184 | public function setTimeLimit($dateMin = null, $dateMax = null, $attribute = 'metamessage.date_send') |
||
| 195 | |||
| 196 | /** |
||
| 197 | * Limit the result to a number of datasets. |
||
| 198 | * |
||
| 199 | * @param int $limit oldest date of messages |
||
| 200 | */ |
||
| 201 | public function setResultLimit($limit) |
||
| 205 | |||
| 206 | /** |
||
| 207 | * Sets an order for the query. |
||
| 208 | * |
||
| 209 | * This function overwrites previous values if they were set as ordering by |
||
| 210 | * multiple columns is not possible. |
||
| 211 | * |
||
| 212 | * @param int $column the column to order by (see {@link OrderChoice} for valid values) |
||
| 213 | * @param string $direction asc or desc |
||
| 214 | */ |
||
| 215 | public function setOrder($column, $direction = 'asc') |
||
| 220 | |||
| 221 | /** |
||
| 222 | * Queries all available data from a list of message IDs. |
||
| 223 | * |
||
| 224 | * Note that this requires one to have the meta data of the messages already |
||
| 225 | * and therefore you have to run {@link getMessageMetaData()} before and |
||
| 226 | * submit it as the first parameter. |
||
| 227 | * This method also resets the conditions of the where clause |
||
| 228 | * ($fetchOptions['where']) and the params ($fetchOptions['params']) based |
||
| 229 | * on the results included in the meta data. Other fetch options however |
||
| 230 | * remain and are still applied, so if you want to avoid this, use |
||
| 231 | * {@link resetFetchOptions()}. |
||
| 232 | * Note that the ordering values of different message types will not work as |
||
| 233 | * this function internally needs to handle each message type differently. |
||
| 234 | * |
||
| 235 | * @param array[string] $metaData The message meta data from |
||
| 236 | * {@link getMessageMetaData()} |
||
| 237 | * (without grouping) |
||
| 238 | * @param bool $groupByMessageType Set to true to group the |
||
| 239 | * return value via message |
||
| 240 | * types. (default: false) |
||
| 241 | * @throws XenForo_Exception |
||
| 242 | * @return null|array |
||
| 243 | */ |
||
| 244 | public function getAllMessageData(array $metaData, $groupByMessageType = false) |
||
| 302 | |||
| 303 | /** |
||
| 304 | * Queries all available data for a message type. |
||
| 305 | * |
||
| 306 | * The return value should be an array in the same format as the one |
||
| 307 | * returned by {@link getAllMessageData()} when $groupByMessageType is set |
||
| 308 | * to false. Of course, however, only one message type is returned here. |
||
| 309 | * |
||
| 310 | * @param int $messageType The message type the messages belong to |
||
| 311 | * @param bool $includeMetaData Set to true to also include the main |
||
| 312 | * message table in your query. If you do so you |
||
| 313 | * will also get the meta data of the message. |
||
| 314 | * (default: true) |
||
| 315 | * @throws XenForo_Exception |
||
| 316 | * @return null|array |
||
| 317 | */ |
||
| 318 | public function getMessageDataByType($messageType, $includeMetaData = true) |
||
| 458 | |||
| 459 | /** |
||
| 460 | * Returns only the meta data of one or more messages not depending on the |
||
| 461 | * type of the message. |
||
| 462 | * |
||
| 463 | * @param bool $groupById When true groups the data by the message ID (default: false) |
||
| 464 | * @param bool $ignoreInvalid When true removes data sets where the message content may be deleted (default: true) |
||
| 465 | * @return null|array |
||
| 466 | */ |
||
| 467 | public function getMessageMetaData($groupById = false, $ignoreInvalid = true) |
||
| 504 | |||
| 505 | /** |
||
| 506 | * Removes entries or meta data fields from the meta data message table. |
||
| 507 | * |
||
| 508 | * Note that if the message(s) has/have other data saved in the database |
||
| 509 | * the full deletion will fail. |
||
| 510 | * Note: The number of where and params-options must be equal. You can |
||
| 511 | * submit additional conditions with the first parameter. |
||
| 512 | * Attention: This ignores the limit/offset clause for simplicity. |
||
| 513 | * |
||
| 514 | * @param string[] $additionalConditions Add additional where conditions if |
||
| 515 | * neccessary. |
||
| 516 | * @param string[] $removeOnlyField When set only the passed fields are |
||
| 517 | * updated to "null" rather than deleting |
||
| 518 | * the whole record. |
||
| 519 | */ |
||
| 520 | public function removeMetaData(array $additionalConditions = [], array $removeOnlyField = []) |
||
| 541 | |||
| 542 | /** |
||
| 543 | * Returns all available data from a list of message IDs. |
||
| 544 | * |
||
| 545 | * @param array[string] $messages The message result |
||
| 546 | * @throws XenForo_Exception |
||
| 547 | * @return null|array |
||
| 548 | */ |
||
| 549 | protected function getMessageIdsFromResult(array $messages) |
||
| 564 | |||
| 565 | /** |
||
| 566 | * Removes the specified keys from the second array and pushes them into |
||
| 567 | * the first base array. |
||
| 568 | * The subarray must be indexed by integers, where each index contains an |
||
| 569 | * associative array with the keys to remove. |
||
| 570 | * It assumes that the 0-index of $subArray is there, including the data, |
||
| 571 | * which should be pushed to $baseArray. |
||
| 572 | * |
||
| 573 | * @param array $baseArray the main array, where the key/value pairs get to |
||
| 574 | * @param array $subArray the array, which keys should be removed |
||
| 575 | * @param string[] $removeKeys an array of keys, which should be removed |
||
| 576 | * |
||
| 577 | * @throws XenForo_Exception |
||
| 578 | * @return false|array |
||
| 579 | */ |
||
| 580 | protected function pushArrayKeys(array &$baseArray, array &$subArray, array $removeKeys) |
||
| 601 | |||
| 602 | /** |
||
| 603 | * Groups an array by using the value of a specific index in it. |
||
| 604 | * |
||
| 605 | * @param array $array the array, which is sued as the base |
||
| 606 | * @param string|int $indexKey the value of the key, which should be used |
||
| 607 | * for indexing |
||
| 608 | * @param bool $ignoreIndex Set to true to ignore multiple values in |
||
| 609 | * $array. If activated only the last key of |
||
| 610 | * $array will be placed into the group and |
||
| 611 | * it will be the only key. This is only |
||
| 612 | * useful if you know for sure that only one |
||
| 613 | * key is available. |
||
| 614 | * |
||
| 615 | * @return array |
||
| 616 | */ |
||
| 617 | public function groupArray(array $array, $indexKey, $ignoreIndex = false) |
||
| 631 | |||
| 632 | /** |
||
| 633 | * Appends a WHERE condition for either a string or an array. |
||
| 634 | * |
||
| 635 | * It automatically chooses between a simple `this = ?` or a more complex |
||
| 636 | * `this IN (?, ?, ...)`. |
||
| 637 | * |
||
| 638 | * @param string $attName the name of the required attribut |
||
| 639 | * @param string|array $attValue the value, which should be required |
||
| 640 | * |
||
| 641 | * @return array |
||
| 642 | */ |
||
| 643 | protected function appendMixedCondition($attName, $attValue) |
||
| 658 | } |
||
| 659 |
In PHP, under loose comparison (like
==, or!=, orswitchconditions), values of different types might be equal.For
integervalues, zero is a special case, in particular the following results might be unexpected: