Complex classes like SendMessages 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 SendMessages, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
27 | class SendMessages extends SendMessagesAbstract |
||
28 | { |
||
29 | protected $template = 'update_issue'; |
||
30 | |||
31 | /** |
||
32 | * Collection of tags. |
||
33 | * |
||
34 | * @var Collection |
||
35 | */ |
||
36 | protected $tags; |
||
37 | |||
38 | /** |
||
39 | * Returns an instance of Issue. |
||
40 | * |
||
41 | * @return ProjectIssue |
||
42 | */ |
||
43 | 4 | protected function getIssue() |
|
51 | |||
52 | /** |
||
53 | * Returns an instance of Project. |
||
54 | * |
||
55 | * @return Project |
||
56 | */ |
||
57 | 4 | protected function getProject() |
|
61 | |||
62 | /** |
||
63 | * Returns the project id. |
||
64 | * |
||
65 | * @return int |
||
66 | */ |
||
67 | 4 | protected function getProjectId() |
|
71 | |||
72 | /** |
||
73 | * Returns message data belongs to adding an issue. |
||
74 | * |
||
75 | * @param Queue $queue |
||
76 | * |
||
77 | * @return array |
||
78 | */ |
||
79 | 4 | protected function getMessageDataForAddIssue(Queue $queue) |
|
80 | { |
||
81 | 4 | $messageData = ['changes' => []]; |
|
82 | 4 | $messageData['changeByHeading'] = $queue->changeBy->fullname . ' created a new issue'; |
|
83 | 4 | if ($this->issue->assigned) { |
|
84 | 2 | $messageData['changes']['assignee'] = $this->issue->assigned->fullname; |
|
85 | } |
||
86 | |||
87 | 4 | $tags = $this->issue->tags()->with('parent')->get(); |
|
88 | 4 | foreach ($tags as $tag) { |
|
89 | 2 | $tagArray = $tag->toShortArray(); |
|
90 | 2 | $tagArray['now'] = $tagArray['name']; |
|
91 | 2 | $messageData['changes'][$tag->parent->name] = $tagArray; |
|
92 | } |
||
93 | |||
94 | 4 | if ($this->issue->time_quote && !$this->issue->isQuoteLocked()) { |
|
95 | $messageData['changes']['time_quote'] = \Html::duration($this->issue->time_quote); |
||
96 | } |
||
97 | |||
98 | 4 | return $messageData; |
|
99 | } |
||
100 | |||
101 | /** |
||
102 | * Returns message data belongs to updating an issue. |
||
103 | * |
||
104 | * @param Queue $queue |
||
105 | * |
||
106 | * @return array |
||
107 | */ |
||
108 | 1 | protected function getMessageDataForUpdateIssue(Queue $queue) |
|
109 | { |
||
110 | 1 | $messageData = []; |
|
111 | 1 | $messageData['changeByHeading'] = $queue->changeBy->fullname . ' updated an issue'; |
|
112 | 1 | $whiteListFields = ['change_by', 'title', 'body', 'assignee', 'status', 'type', 'resolution', 'time_quote']; |
|
113 | |||
114 | 1 | foreach ($queue->payload['dirty'] as $field => $value) { |
|
115 | // Skip fields that not part of the white list or quote is locked |
||
116 | 1 | if (!in_array($field, $whiteListFields) || ($field == 'time_quote' && $this->issue->isQuoteLocked())) { |
|
117 | 1 | continue; |
|
118 | } |
||
119 | |||
120 | // Format quote to readable time |
||
121 | 1 | $value = $field === 'time_quote' ? \Html::duration($value) : $value; |
|
122 | 1 | $value = $field === 'body' ? \Html::format($value) : $value; |
|
123 | 1 | $messageData['changes'][$field] = [ |
|
124 | 1 | 'now' => $value, |
|
125 | 1 | 'was' => $queue->getDataFromPayload('origin.' . $field), |
|
126 | ]; |
||
127 | } |
||
128 | |||
129 | 1 | return $messageData; |
|
130 | } |
||
131 | |||
132 | /** |
||
133 | * Returns message data belongs to reopening an issue. |
||
134 | * |
||
135 | * @param Queue $queue |
||
136 | * |
||
137 | * @return array |
||
138 | */ |
||
139 | 1 | protected function getMessageDataForReopenIssue(Queue $queue) |
|
153 | |||
154 | /** |
||
155 | * Returns message data belongs to closing an issue. |
||
156 | * |
||
157 | * @param Queue $queue |
||
158 | * |
||
159 | * @return array |
||
160 | */ |
||
161 | 2 | protected function getMessageDataForCloseIssue(Queue $queue) |
|
174 | |||
175 | /** |
||
176 | * Returns message data belongs to assigning an issue to a user. |
||
177 | * |
||
178 | * @param Queue $queue |
||
179 | * @param array $extraData |
||
180 | * |
||
181 | * @return array |
||
182 | */ |
||
183 | 1 | protected function getMessageDataForAssignIssue(Queue $queue, array $extraData) |
|
196 | |||
197 | /** |
||
198 | * Returns message data belongs to changing an issue tags. |
||
199 | * |
||
200 | * @param Queue $queue |
||
201 | * |
||
202 | * @return array |
||
203 | */ |
||
204 | 3 | protected function getMessageDataForChangeTagIssue(Queue $queue) |
|
225 | |||
226 | /** |
||
227 | * Check that the issue is load. |
||
228 | * |
||
229 | * @return bool |
||
230 | */ |
||
231 | 4 | protected function validateData() |
|
240 | |||
241 | /** |
||
242 | * Populate assigned relation in the current issue. |
||
243 | * |
||
244 | * @return void |
||
245 | */ |
||
246 | 4 | protected function populateData() |
|
1 ignored issue
–
show
|
|||
247 | { |
||
248 | 4 | $this->issue->setRelation('assigned', $this->getUserById($this->issue->assigned_to)); |
|
249 | 4 | $creator = $this->getUserById($this->issue->created_by); |
|
250 | 4 | if ($creator) { |
|
251 | 4 | $this->issue->setRelation('user', $creator); |
|
252 | } |
||
253 | 4 | $this->loadIssueCreatorToProjectUsers(); |
|
254 | 4 | } |
|
255 | |||
256 | /** |
||
257 | * Check if the latest message is about closing or reopening an issue. |
||
258 | * |
||
259 | * @return bool |
||
260 | */ |
||
261 | 4 | public function isStatusMessage() |
|
266 | |||
267 | /** |
||
268 | * Process assign to user message. Send direct message to new and previous users and full subscribers. |
||
269 | * |
||
270 | * @return void |
||
271 | */ |
||
272 | 4 | protected function processDirectMessages() |
|
273 | { |
||
274 | // Stop if issue is closed |
||
275 | 4 | if (!$this->getIssue()->isOpen()) { |
|
276 | 2 | return; |
|
277 | } |
||
278 | |||
279 | // Fetch all of the assign issue changes |
||
280 | 4 | $assignMessages = $this->allMessages->where('event', Queue::ASSIGN_ISSUE); |
|
281 | |||
282 | // Skip if no changes |
||
283 | 4 | if ($assignMessages->isEmpty()) { |
|
284 | 4 | return; |
|
285 | } |
||
286 | |||
287 | // Fetch the latest assignee |
||
288 | /** @var Queue $assignMessage */ |
||
289 | 1 | $assignMessage = $assignMessages->first(); |
|
290 | |||
291 | // Fetch the user details of the new assignee & previous assignee if this isn't new issue |
||
292 | 1 | $assigns = []; |
|
293 | 1 | $assigns['new'] = (int) $assignMessage->getDataFromPayload('dirty.assigned_to'); |
|
294 | 1 | if (!$this->addMessage) { |
|
295 | 1 | $previousAssignMessage = $assignMessages->last(); |
|
296 | 1 | $assigns['old'] = (int) $previousAssignMessage->getDataFromPayload('origin.assigned_to'); |
|
297 | } |
||
298 | |||
299 | // Fetch users objects for old and new assignee |
||
300 | /** @var \Illuminate\Database\Eloquent\Collection $assignObjects */ |
||
301 | 1 | $assignObjects = (new User())->whereIn('id', $assigns)->get(); |
|
302 | |||
303 | // If for what ever reason the user does not exists, skip this change |
||
304 | // or if there is only one user and is not matching the new assignee. |
||
305 | // then skip this message |
||
306 | 1 | if ($assignObjects->count() === 0 |
|
307 | 1 | || ($assignObjects->count() === 1 && (int) $assignObjects->first()->id !== $assigns['new']) |
|
308 | ) { |
||
309 | return; |
||
310 | } |
||
311 | |||
312 | // Get the object of the new assignee |
||
313 | 1 | $assignTo = $assignObjects->where('id', $assigns['new'], false)->first(); |
|
314 | 1 | $users = collect([$this->createProjectUserObject($assignTo)]); |
|
315 | |||
316 | // Exclude the user from any other message for this issue |
||
317 | 1 | $this->addToExcludeUsers($assignTo); |
|
318 | |||
319 | // Data about new and previous assignee |
||
320 | $extraMessageData = [ |
||
321 | 1 | 'now' => $assignTo->fullname, |
|
322 | ]; |
||
323 | |||
324 | // Make sure that the previous assignee was not the same as the new user |
||
325 | 1 | if (array_key_exists('old', $assigns) && $assigns['old'] > 0 && $assigns['new'] !== $assigns['old']) { |
|
326 | $previousAssign = $assignObjects->where('id', $assigns['old'], false)->first(); |
||
327 | if ($previousAssign) { |
||
328 | $extraMessageData['was'] = $previousAssign->fullname; |
||
329 | $users->push($this->createProjectUserObject($previousAssign)); |
||
330 | } |
||
331 | } |
||
332 | |||
333 | // Get message data needed for the message & send |
||
334 | 1 | $messageData = $this->getMessageData($assignMessage, $extraMessageData); |
|
335 | 1 | $this->sendMessages($users, $messageData); |
|
336 | 1 | } |
|
337 | |||
338 | /** |
||
339 | * Create user project object for a user. |
||
340 | * |
||
341 | * @param User $user |
||
342 | * |
||
343 | * @return Project\User |
||
344 | */ |
||
345 | 1 | protected function createProjectUserObject(User $user) |
|
356 | |||
357 | /** |
||
358 | * Get collection of tags or one by ID. |
||
359 | * |
||
360 | * @param int $tagId |
||
361 | * |
||
362 | * @return Tag |
||
363 | */ |
||
364 | 3 | protected function getTag($tagId) |
|
379 | |||
380 | /** |
||
381 | * Whether or not the user wants to receive the message. |
||
382 | * |
||
383 | * @param Project\User $user |
||
384 | * @param array $data |
||
385 | * |
||
386 | * @return bool |
||
387 | */ |
||
388 | 4 | protected function wantToReceiveMessage(Project\User $user, array $data) |
|
424 | } |
||
425 |
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.