allejo /
bzion
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 Conversation 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 Conversation, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 13 | class Conversation extends UrlModel implements NamedModel |
||
| 14 | { |
||
| 15 | /** |
||
| 16 | * The subject of the conversation |
||
| 17 | * @var string |
||
| 18 | */ |
||
| 19 | protected $subject; |
||
| 20 | |||
| 21 | /** |
||
| 22 | * The time of the last message to the conversation |
||
| 23 | * @var TimeDate |
||
| 24 | */ |
||
| 25 | protected $last_activity; |
||
| 26 | |||
| 27 | /** |
||
| 28 | * The id of the creator of the conversation |
||
| 29 | * @var int |
||
| 30 | */ |
||
| 31 | protected $creator; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * The status of the conversation |
||
| 35 | * |
||
| 36 | * Can be 'active', 'disabled', 'deleted' or 'reported' |
||
| 37 | * @var string |
||
| 38 | */ |
||
| 39 | protected $status; |
||
| 40 | |||
| 41 | /** |
||
| 42 | * The name of the database table used for queries |
||
| 43 | */ |
||
| 44 | const TABLE = "conversations"; |
||
| 45 | |||
| 46 | /** |
||
| 47 | * {@inheritdoc} |
||
| 48 | */ |
||
| 49 | 1 | protected function assignResult($conversation) |
|
| 50 | { |
||
| 51 | 1 | $this->subject = $conversation['subject']; |
|
| 52 | 1 | $this->last_activity = TimeDate::fromMysql($conversation['last_activity']); |
|
| 53 | 1 | $this->creator = $conversation['creator']; |
|
| 54 | 1 | $this->status = $conversation['status']; |
|
| 55 | 1 | } |
|
| 56 | |||
| 57 | /** |
||
| 58 | * Get the subject of the discussion |
||
| 59 | * |
||
| 60 | * @return string |
||
| 61 | **/ |
||
| 62 | 1 | public function getSubject() |
|
| 63 | { |
||
| 64 | 1 | return $this->subject; |
|
| 65 | } |
||
| 66 | |||
| 67 | /** |
||
| 68 | * Get the creator of the discussion |
||
| 69 | * |
||
| 70 | * @return Player |
||
| 71 | */ |
||
| 72 | 1 | public function getCreator() |
|
| 73 | { |
||
| 74 | 1 | return Player::get($this->creator); |
|
| 75 | } |
||
| 76 | |||
| 77 | /** |
||
| 78 | * Determine whether a player is the one who created the message conversation |
||
| 79 | * |
||
| 80 | * @param int $id The ID of the player to test for |
||
| 81 | * @return bool |
||
| 82 | */ |
||
| 83 | 1 | public function isCreator($id) |
|
| 84 | { |
||
| 85 | 1 | return $this->creator == $id; |
|
| 86 | } |
||
| 87 | |||
| 88 | /** |
||
| 89 | * {@inheritdoc} |
||
| 90 | */ |
||
| 91 | 1 | public function isEditor($player) |
|
| 92 | { |
||
| 93 | 1 | return $this->isCreator($player->getId()); |
|
| 94 | } |
||
| 95 | |||
| 96 | /** |
||
| 97 | * Get the time when the conversation was most recently active |
||
| 98 | * |
||
| 99 | * @return TimeDate |
||
| 100 | */ |
||
| 101 | public function getLastActivity() |
||
| 102 | { |
||
| 103 | return $this->last_activity->copy(); |
||
| 104 | } |
||
| 105 | |||
| 106 | /** |
||
| 107 | * Update the conversation's last activity timestamp |
||
| 108 | * |
||
| 109 | * @return void |
||
| 110 | */ |
||
| 111 | 1 | public function updateLastActivity() |
|
| 112 | { |
||
| 113 | 1 | $this->last_activity = TimeDate::now(); |
|
| 114 | 1 | $this->update('last_activity', $this->last_activity->toMysql(), 's'); |
|
| 115 | 1 | } |
|
| 116 | |||
| 117 | /** |
||
| 118 | * Update the conversation's subject |
||
| 119 | * |
||
| 120 | * @param string $subject The new subject |
||
| 121 | * @return self |
||
| 122 | */ |
||
| 123 | public function setSubject($subject) |
||
| 124 | { |
||
| 125 | return $this->updateProperty($this->subject, 'subject', $subject, 's'); |
||
| 126 | } |
||
| 127 | |||
| 128 | /** |
||
| 129 | * Get the last message of the conversation |
||
| 130 | * |
||
| 131 | * @return Message |
||
| 132 | */ |
||
| 133 | public function getLastMessage() |
||
| 134 | { |
||
| 135 | $ids = self::fetchIdsFrom('conversation_to', array($this->id), 'i', false, 'AND event_type IS null ORDER BY id DESC LIMIT 0,1', 'messages'); |
||
| 136 | |||
| 137 | if (!isset($ids[0])) { |
||
| 138 | return Message::invalid(); |
||
| 139 | } |
||
| 140 | |||
| 141 | return Message::get($ids[0]); |
||
| 142 | } |
||
| 143 | |||
| 144 | /** |
||
| 145 | * Find whether the last message in the conversation has been read by a player |
||
| 146 | * |
||
| 147 | * @param int $playerId The ID of the player |
||
| 148 | * @return bool |
||
| 149 | */ |
||
| 150 | View Code Duplication | public function isReadBy($playerId) |
|
|
|
|||
| 151 | { |
||
| 152 | $query = $this->db->query("SELECT `read` FROM `player_conversations` WHERE `player` = ? AND `conversation` = ?", |
||
| 153 | 'ii', array($playerId, $this->id)); |
||
| 154 | |||
| 155 | return $query[0]['read'] == 1; |
||
| 156 | } |
||
| 157 | |||
| 158 | /** |
||
| 159 | * Mark the last message in the conversation as having been read by a player |
||
| 160 | * |
||
| 161 | * @param int $playerId The ID of the player |
||
| 162 | * @return void |
||
| 163 | */ |
||
| 164 | 1 | public function markReadBy($playerId) |
|
| 165 | { |
||
| 166 | 1 | $this->db->query( |
|
| 167 | 1 | "UPDATE `player_conversations` SET `read` = 1 WHERE `player` = ? AND `conversation` = ? AND `read` = 0", |
|
| 168 | 1 | 'ii', array($playerId, $this->id) |
|
| 169 | ); |
||
| 170 | 1 | } |
|
| 171 | |||
| 172 | /** |
||
| 173 | * Mark the last message in the conversation as unread by the conversation's members |
||
| 174 | * |
||
| 175 | * @param int $except The ID of a player to exclude |
||
| 176 | * @return void |
||
| 177 | */ |
||
| 178 | 1 | public function markUnread($except) |
|
| 179 | { |
||
| 180 | 1 | $this->db->query( |
|
| 181 | 1 | "UPDATE `player_conversations` SET `read` = 0 WHERE `conversation` = ? AND `player` != ?", |
|
| 182 | 1 | 'ii', |
|
| 183 | 1 | array($this->id, $except) |
|
| 184 | ); |
||
| 185 | 1 | } |
|
| 186 | |||
| 187 | /** |
||
| 188 | * {@inheritdoc} |
||
| 189 | */ |
||
| 190 | 1 | public static function getRouteName($action = 'show') |
|
| 191 | { |
||
| 192 | 1 | return "message_conversation_$action"; |
|
| 193 | } |
||
| 194 | |||
| 195 | /** |
||
| 196 | * {@inheritdoc} |
||
| 197 | */ |
||
| 198 | 1 | public static function getActiveStatuses() |
|
| 199 | { |
||
| 200 | 1 | return array('active', 'reported'); |
|
| 201 | } |
||
| 202 | |||
| 203 | /** |
||
| 204 | * {@inheritdoc} |
||
| 205 | */ |
||
| 206 | 1 | public function getName() |
|
| 207 | { |
||
| 208 | 1 | return $this->getSubject(); |
|
| 209 | } |
||
| 210 | |||
| 211 | /** |
||
| 212 | * Get a list containing each member of the conversation |
||
| 213 | * @param int|null $hide The ID of a player to ignore |
||
| 214 | * @return Model[] An array of players and teams |
||
| 215 | */ |
||
| 216 | 1 | public function getMembers($hide = null) |
|
| 217 | { |
||
| 218 | 1 | $members = Player::arrayIdToModel($this->getPlayerIds($hide, true)); |
|
| 219 | 1 | usort($members, Player::getAlphabeticalSort()); |
|
| 220 | |||
| 221 | 1 | $teams = Team::arrayIdToModel($this->getTeamIds()); |
|
| 222 | 1 | usort($teams, Team::getAlphabeticalSort()); |
|
| 223 | |||
| 224 | 1 | return array_merge($members, $teams); |
|
| 225 | } |
||
| 226 | |||
| 227 | /** |
||
| 228 | * Get the members of one of the conversation's teams that don't belong in |
||
| 229 | * the conversation |
||
| 230 | * |
||
| 231 | * @todo Use Model::createFromDatabaseResults() |
||
| 232 | * @param Team $team The team to check |
||
| 233 | * @return Player[] |
||
| 234 | */ |
||
| 235 | public function getMissingTeamMembers(Team $team) |
||
| 248 | |||
| 249 | /** |
||
| 250 | * Get a list containing the IDs of each member player of the conversation |
||
| 251 | * @param int|null $hide The ID of a player to ignore |
||
| 252 | * @param bool $distinct Whether to only return players who were |
||
| 253 | * specifically invited to the conversation, and |
||
| 254 | * are not participating only as members of a team |
||
| 255 | * @return int[] An array of player IDs |
||
| 256 | */ |
||
| 257 | 1 | public function getPlayerIds($hide = null, $distinct = false) |
|
| 258 | { |
||
| 259 | 1 | $additional_query = "WHERE `conversation` = ?"; |
|
| 260 | 1 | $types = "i"; |
|
| 261 | 1 | $params = array($this->id); |
|
| 262 | |||
| 263 | 1 | if ($hide) { |
|
| 264 | $additional_query .= " AND `player` != ?"; |
||
| 265 | $types .= "i"; |
||
| 266 | $params[] = $hide; |
||
| 267 | } |
||
| 268 | |||
| 269 | 1 | if ($distinct) { |
|
| 270 | 1 | $additional_query .= " AND `distinct` = 1"; |
|
| 271 | } |
||
| 272 | |||
| 273 | 1 | return parent::fetchIds($additional_query, $types, $params, "player_conversations", "player"); |
|
| 274 | } |
||
| 275 | |||
| 276 | /** |
||
| 277 | * Get a list containing the IDs of each member team of the conversation |
||
| 278 | * |
||
| 279 | * @return int[] An array of team IDs |
||
| 280 | */ |
||
| 281 | 1 | public function getTeamIds() |
|
| 282 | { |
||
| 283 | 1 | return parent::fetchIds("WHERE `conversation` = ?", "i", $this->id, "team_conversations", "team"); |
|
| 284 | } |
||
| 285 | |||
| 286 | /** |
||
| 287 | * Create a new message conversation |
||
| 288 | ** |
||
| 289 | * @param string $subject The subject of the conversation |
||
| 290 | * @param int $creatorId The ID of the player who created the conversation |
||
| 291 | * @param array $members A list of Models representing the conversation's members |
||
| 292 | * @return Conversation An object that represents the created conversation |
||
| 293 | */ |
||
| 294 | 1 | public static function createConversation($subject, $creatorId, $members = array()) |
|
| 310 | |||
| 311 | /** |
||
| 312 | * Send a new message to the conversation's members |
||
| 313 | * @param Player $from The sender |
||
| 314 | * @param string $message The body of the message |
||
| 315 | * @param string $status The status of the message - can be 'visible', 'hidden', 'deleted' or 'reported' |
||
| 316 | * @return Message An object that represents the sent message |
||
| 317 | */ |
||
| 318 | 1 | public function sendMessage($from, $message, $status = 'visible') |
|
| 319 | { |
||
| 320 | 1 | $message = Message::sendMessage($this->getId(), $from->getId(), $message, $status); |
|
| 321 | |||
| 322 | 1 | $this->updateLastActivity(); |
|
| 323 | |||
| 324 | 1 | return $message; |
|
| 325 | } |
||
| 326 | |||
| 327 | /** |
||
| 328 | * Checks if a player or team belongs in the conversation |
||
| 329 | * @param Player|Team $member The player or team to check |
||
| 330 | * @param bool Whether to only return true if a player is specifically a |
||
| 331 | * member of the conversation, not just a member of one of the |
||
| 332 | * conversation's teams (ignored if $member is a Team) |
||
| 333 | * @return bool True if the given object belongs in the conversation, false if they don't |
||
| 334 | */ |
||
| 335 | 1 | public function isMember($member, $distinct = false) |
|
| 352 | |||
| 353 | /** |
||
| 354 | * Add a member to the discussion |
||
| 355 | * |
||
| 356 | * @param Player|Team $member The member to add |
||
| 357 | * @param bool $distinct Whether to add the member as a distinct |
||
| 358 | * player (ignored for teams) |
||
| 359 | * @return void |
||
| 360 | */ |
||
| 361 | 1 | public function addMember($member, $distinct = true) |
|
| 393 | |||
| 394 | /** |
||
| 395 | * Find out if a player belongs to any of the conversation's teams |
||
| 396 | * |
||
| 397 | * This does not take into account whether the player is a distinct member |
||
| 398 | * of the conversation (i.e. they have been invited separately) |
||
| 399 | * |
||
| 400 | * @param Player $member The player to check |
||
| 401 | * @return bool |
||
| 402 | */ |
||
| 403 | 1 | View Code Duplication | public function isTeamMember($member) |
| 416 | |||
| 417 | /** |
||
| 418 | * Remove a member from the discussion |
||
| 419 | * |
||
| 420 | * @param Player|Team $member The member to remove |
||
| 421 | * @return void |
||
| 422 | */ |
||
| 423 | public function removeMember($member) |
||
| 453 | |||
| 454 | /** |
||
| 455 | * Find out which members of the conversation should receive an e-mail after a new |
||
| 456 | * message has been sent |
||
| 457 | * |
||
| 458 | * @param int $except The ID of a player who won't receive an e-mail |
||
| 459 | * (e.g. message author) |
||
| 460 | * @param bool $read Whether to only send e-mails to players who have |
||
| 461 | * read all the previous messages in the conversation |
||
| 462 | * @return int[] A player ID list |
||
| 463 | */ |
||
| 464 | 1 | public function getWaitingForEmailIDs($except, $read = true) |
|
| 480 | |||
| 481 | /** |
||
| 482 | * Get a query builder for conversations |
||
| 483 | * @return ConversationQueryBuilder |
||
| 484 | */ |
||
| 485 | 1 | public static function getQueryBuilder() |
|
| 495 | } |
||
| 496 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.