Complex classes like SendMessagesAbstract 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 SendMessagesAbstract, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
28 | abstract class SendMessagesAbstract |
||
29 | { |
||
30 | /** |
||
31 | * Instance of issue that this message belong to. |
||
32 | * |
||
33 | * @var Issue |
||
34 | */ |
||
35 | protected $issue; |
||
36 | |||
37 | /** |
||
38 | * Instance of project that this message belong to. |
||
39 | * |
||
40 | * @var Issue |
||
41 | */ |
||
42 | protected $project; |
||
43 | |||
44 | /** |
||
45 | * The latest message queued. |
||
46 | * |
||
47 | * @var Message\Queue |
||
48 | */ |
||
49 | protected $latestMessage; |
||
50 | |||
51 | /** |
||
52 | * Collection of all of the queued messages. |
||
53 | * |
||
54 | * @var Collection |
||
55 | */ |
||
56 | protected $allMessages; |
||
57 | |||
58 | /** |
||
59 | * Instance of a queued message that is for adding a record (ie. adding issue). |
||
60 | * |
||
61 | * @var Message\Queue |
||
62 | */ |
||
63 | protected $addMessage; |
||
64 | /** |
||
65 | * Collection of users that must not receive messages. |
||
66 | * |
||
67 | * @var Collection |
||
68 | */ |
||
69 | protected $excludeUsers; |
||
70 | /** |
||
71 | * Collection of all of the project users that should receive messages. |
||
72 | * |
||
73 | * @var Collection |
||
74 | */ |
||
75 | protected $projectUsers; |
||
76 | /** |
||
77 | * Collection of full subscribers that will always receive messages. |
||
78 | * |
||
79 | * @var Collection |
||
80 | */ |
||
81 | protected $fullSubscribers; |
||
82 | /** |
||
83 | * Name of message template. |
||
84 | * |
||
85 | * @var string |
||
86 | */ |
||
87 | protected $template; |
||
88 | |||
89 | /** |
||
90 | * @var Mailer |
||
91 | */ |
||
92 | protected $mailer; |
||
93 | |||
94 | /** |
||
95 | * Collection of all messages. |
||
96 | * |
||
97 | * @var Collection |
||
98 | */ |
||
99 | protected $messages; |
||
100 | |||
101 | /** |
||
102 | * Set instance of Mailer. |
||
103 | * |
||
104 | * @param Mailer $mailer |
||
105 | * |
||
106 | * @return $this |
||
107 | */ |
||
108 | 4 | public function setMailer(Mailer $mailer) |
|
114 | |||
115 | /** |
||
116 | * The main method to process the massages queue and send them. |
||
117 | * |
||
118 | * @param Message\Queue $latestMessage |
||
119 | * @param Collection $changes |
||
120 | * |
||
121 | * @return void |
||
122 | */ |
||
123 | 4 | public function process(Message\Queue $latestMessage, Collection $changes) |
|
124 | { |
||
125 | 4 | $this->setup($latestMessage, $changes); |
|
126 | |||
127 | // Is model deleted? |
||
128 | 4 | if (!$this->getModel()) { |
|
129 | 2 | return $this->sendMessageToAll($this->latestMessage); |
|
130 | } |
||
131 | |||
132 | 4 | if (!$this->validateData()) { |
|
133 | return; |
||
134 | } |
||
135 | |||
136 | 4 | $this->processDirectMessages(); |
|
137 | |||
138 | // Skip if no users found |
||
139 | 4 | if ($this->getProjectUsers()->isEmpty()) { |
|
140 | return; |
||
141 | } |
||
142 | |||
143 | 4 | $this->populateData(); |
|
144 | |||
145 | // Send the latest message if it is about status (ie. closed issue) |
||
146 | 4 | if ($this->isStatusMessage()) { |
|
147 | return $this->sendMessageToAll($this->latestMessage); |
||
148 | } |
||
149 | |||
150 | // Get message data for all of the messages combined & for add message queue if exists |
||
151 | 4 | $addMessageData = []; |
|
152 | 4 | if ($this->addMessage) { |
|
153 | 4 | $addMessageData = $this->getMessageData($this->addMessage); |
|
154 | } |
||
155 | 4 | $everythingMessageData = $this->getCombineMessageData($this->allMessages); |
|
156 | |||
157 | // Send messages to project users |
||
158 | 4 | $this->sendMessages($this->getProjectUsers(), [ |
|
159 | 4 | 'addMessage' => $addMessageData, |
|
160 | 4 | 'everything' => $everythingMessageData, |
|
161 | ]); |
||
162 | 4 | } |
|
163 | |||
164 | /** |
||
165 | * Setup properties needed for the process. |
||
166 | * |
||
167 | * @param Message\Queue $latestMessage |
||
168 | * @param Collection $allMessages |
||
169 | * |
||
170 | * @return void |
||
171 | */ |
||
172 | 4 | protected function setup(Message\Queue $latestMessage, Collection $allMessages) |
|
173 | { |
||
174 | // Set queue messages |
||
175 | 4 | $this->latestMessage = $latestMessage; |
|
176 | 4 | $this->allMessages = $allMessages; |
|
177 | |||
178 | // Exclude the user who made the change from receiving messages |
||
179 | 4 | $this->addToExcludeUsers($this->latestMessage->changeBy); |
|
|
|||
180 | |||
181 | // Extract add model message if exists |
||
182 | 4 | if ($this->getModel()) { |
|
183 | 4 | $addMessageIdentifier = Message\Queue::getAddEventNameFromModel($this->getModel()); |
|
184 | 4 | $this->addMessage = $this->allMessages->where('event', $addMessageIdentifier)->first(); |
|
185 | } |
||
186 | |||
187 | // Make sure to load issue |
||
188 | 4 | $this->getIssue(); |
|
189 | 4 | } |
|
190 | |||
191 | /** |
||
192 | * Whether or not we have all the needed properties. |
||
193 | * |
||
194 | * @return bool |
||
195 | */ |
||
196 | abstract protected function validateData(); |
||
197 | |||
198 | /** |
||
199 | * Process any messages queue that is to send messages to specific users. |
||
200 | * For example, assign issue to user to message the user about the issue. |
||
201 | * |
||
202 | * @return void |
||
203 | */ |
||
204 | 2 | protected function processDirectMessages() |
|
205 | { |
||
206 | 2 | } |
|
207 | |||
208 | /** |
||
209 | * Populate any data or properties. |
||
210 | * |
||
211 | * @return void |
||
212 | */ |
||
213 | 1 | protected function populateData() |
|
214 | { |
||
215 | 1 | } |
|
216 | |||
217 | /** |
||
218 | * Whether or not the latest message is about status change such as closed issue. |
||
219 | * |
||
220 | * @return bool |
||
221 | */ |
||
222 | abstract public function isStatusMessage(); |
||
223 | |||
224 | /** |
||
225 | * Returns the message subject. |
||
226 | * |
||
227 | * @return string |
||
228 | */ |
||
229 | 3 | protected function getSubject() |
|
233 | |||
234 | /** |
||
235 | * Returns an array of data needed for the message. |
||
236 | * |
||
237 | * @param Message\Queue $queue |
||
238 | * @param array $extraData |
||
239 | * |
||
240 | * @return array |
||
241 | */ |
||
242 | 4 | protected function getMessageData(Message\Queue $queue, array $extraData = []) |
|
269 | |||
270 | /** |
||
271 | * Loop through all of the messages and combine its message data. |
||
272 | * |
||
273 | * @param Collection $changes |
||
274 | * |
||
275 | * @return array |
||
276 | */ |
||
277 | 4 | protected function getCombineMessageData(Collection $changes) |
|
296 | |||
297 | /** |
||
298 | * Return text to be used for the message heading. |
||
299 | * |
||
300 | * @param Message\Queue $queue |
||
301 | * @param Collection|null $changes |
||
302 | * |
||
303 | * @return string |
||
304 | */ |
||
305 | 4 | protected function getMessageHeading(Message\Queue $queue, Collection $changes = null) |
|
316 | |||
317 | /** |
||
318 | * Returns collection of all users in a project that should receive the messages. |
||
319 | * |
||
320 | * @return Collection |
||
321 | */ |
||
322 | 4 | protected function getProjectUsers() |
|
334 | |||
335 | /** |
||
336 | * Returns the model that is belong to the queue message. |
||
337 | * |
||
338 | * @return \Illuminate\Database\Eloquent\Model |
||
339 | */ |
||
340 | 4 | protected function getModel() |
|
344 | |||
345 | /** |
||
346 | * Returns an instance of project issue. |
||
347 | * |
||
348 | * @return Issue |
||
349 | */ |
||
350 | abstract protected function getIssue(); |
||
351 | |||
352 | /** |
||
353 | * Returns an instance of project. |
||
354 | * |
||
355 | * @return Project |
||
356 | */ |
||
357 | abstract protected function getProject(); |
||
358 | |||
359 | /** |
||
360 | * Returns the id of a project. |
||
361 | * |
||
362 | * @return int |
||
363 | */ |
||
364 | abstract protected function getProjectId(); |
||
365 | |||
366 | /** |
||
367 | * Returns collection of all of the users that must not receive messages. |
||
368 | * |
||
369 | * @return Collection |
||
370 | */ |
||
371 | 4 | protected function getExcludeUsers() |
|
379 | |||
380 | /** |
||
381 | * Exclude a user from receiving messages. |
||
382 | * |
||
383 | * @param User $user |
||
384 | * |
||
385 | * @return $this |
||
386 | */ |
||
387 | 4 | protected function addToExcludeUsers(User $user) |
|
393 | |||
394 | /** |
||
395 | * Find user by id. This search the project users and fallback to excluded list of users. |
||
396 | * |
||
397 | * @param int $userId |
||
398 | * |
||
399 | * @return User |
||
400 | */ |
||
401 | 3 | protected function getUserById($userId) |
|
411 | |||
412 | /** |
||
413 | * Returns collection of all messages. |
||
414 | * |
||
415 | * @return Collection |
||
416 | */ |
||
417 | 4 | protected function getMessages() |
|
425 | |||
426 | /** |
||
427 | * Send a message to a user. |
||
428 | * |
||
429 | * @param User $user |
||
430 | * @param array $data |
||
431 | * |
||
432 | * @return mixed |
||
433 | */ |
||
434 | 4 | private function sendMessage(User $user, array $data) |
|
445 | |||
446 | /** |
||
447 | * Send a message to a collection of users, or send customised message per use logic. |
||
448 | * |
||
449 | * @param Collection $users |
||
450 | * @param array $data |
||
451 | * |
||
452 | * @return void |
||
453 | */ |
||
454 | 4 | protected function sendMessages(Collection $users, array $data) |
|
455 | { |
||
456 | 4 | foreach ($users as $user) { |
|
457 | 4 | $userMessageData = $this->getUserMessageData($user->user_id, $data); |
|
458 | 4 | if (!$this->wantToReceiveMessage($user, $userMessageData)) { |
|
459 | 4 | continue; |
|
460 | } |
||
461 | |||
462 | 4 | $this->sendMessage($user->user, $userMessageData); |
|
463 | } |
||
464 | 4 | } |
|
465 | |||
466 | /** |
||
467 | * Get customised message per user logic. |
||
468 | * |
||
469 | * @param int $userId |
||
470 | * @param array $messagesData |
||
471 | * |
||
472 | * @return array |
||
473 | */ |
||
474 | 4 | protected function getUserMessageData($userId, array $messagesData) |
|
505 | |||
506 | /** |
||
507 | * Whether or not the user wants to receive the message. |
||
508 | * |
||
509 | * @param Project\User $user |
||
510 | * @param array $data |
||
511 | * |
||
512 | * @return bool |
||
513 | */ |
||
514 | 4 | protected function wantToReceiveMessage(Project\User $user, array $data) |
|
549 | |||
550 | /** |
||
551 | * Send a message to al users in project and full subscribes. |
||
552 | * |
||
553 | * @param Message\Queue $queue |
||
554 | * |
||
555 | * @return void |
||
556 | */ |
||
557 | 2 | protected function sendMessageToAll(Message\Queue $queue) |
|
563 | |||
564 | /** |
||
565 | * Load the creator of an issue to the collection of project users. So we can send message to creator if needed. |
||
566 | * |
||
567 | * @return void |
||
568 | */ |
||
569 | 3 | protected function loadIssueCreatorToProjectUsers() |
|
600 | } |
||
601 |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: