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 SinchService 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 SinchService, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
33 | class SinchService |
||
34 | { |
||
35 | /** |
||
36 | * @var Client $guzzleHTTPClient Guzzle HTTP client |
||
37 | */ |
||
38 | private $guzzleHTTPClient; |
||
39 | |||
40 | /** |
||
41 | * @var string $host Host |
||
42 | */ |
||
43 | private $host; |
||
44 | |||
45 | /** |
||
46 | * @var string $key Key |
||
47 | */ |
||
48 | private $key; |
||
49 | |||
50 | /** |
||
51 | * @var string $secret Secret |
||
52 | */ |
||
53 | private $secret; |
||
54 | |||
55 | /** |
||
56 | * @var string $from From |
||
57 | */ |
||
58 | private $from; |
||
59 | |||
60 | /** |
||
61 | * Constructor |
||
62 | * |
||
63 | * @param string $host Host |
||
64 | * @param string $key Key |
||
65 | * @param string $secret Secret |
||
66 | * @param string|null $from From |
||
67 | */ |
||
68 | public function __construct($host, $key, $secret, $from = null) |
||
79 | |||
80 | // region Public API |
||
81 | |||
82 | /** |
||
83 | * Send SMS |
||
84 | * |
||
85 | * @param string $phoneNumber Phone number |
||
86 | * @param string $messageText Message text |
||
87 | * @param string|null $from From |
||
88 | * |
||
89 | * @return int Message ID |
||
90 | * |
||
91 | * @throws GuzzleException |
||
92 | */ |
||
93 | public function sendSMS($phoneNumber, $messageText, $from = null) |
||
129 | |||
130 | /** |
||
131 | * Get status of sent SMS |
||
132 | * |
||
133 | * Available SMS statuses: Successful, Pending, Faulted, Unknown |
||
134 | * |
||
135 | * @param int $messageId Message ID |
||
136 | * |
||
137 | * @return string SMS status |
||
138 | * |
||
139 | * @throws GuzzleException |
||
140 | */ |
||
141 | public function getStatusOfSMS($messageId) |
||
152 | |||
153 | // endregion |
||
154 | |||
155 | // region Check status helper methods |
||
156 | |||
157 | /** |
||
158 | * Returns true if SMS with some ID was sent successfully, otherwise returns false |
||
159 | * |
||
160 | * @param int $messageId Message ID |
||
161 | * |
||
162 | * @return bool True if SMS was sent successfully, otherwise - false |
||
163 | */ |
||
164 | View Code Duplication | public function smsIsSentSuccessfully($messageId) |
|
175 | |||
176 | /** |
||
177 | * Returns true if SMS with some ID is still pending, otherwise returns false |
||
178 | * |
||
179 | * @param int $messageId Message ID |
||
180 | * |
||
181 | * @return bool True if SMS is still pending, otherwise - false |
||
182 | */ |
||
183 | View Code Duplication | public function smsIsPending($messageId) |
|
194 | |||
195 | /** |
||
196 | * Returns true if SMS with some ID was faulted, otherwise returns false |
||
197 | * |
||
198 | * @param int $messageId Message ID |
||
199 | * |
||
200 | * @return bool True if SMS was faulted, otherwise - false |
||
201 | */ |
||
202 | View Code Duplication | public function smsIsFaulted($messageId) |
|
213 | |||
214 | /** |
||
215 | * Returns true if SMS with some ID in unknown status, otherwise returns false |
||
216 | * |
||
217 | * @param int $messageId Message ID |
||
218 | * |
||
219 | * @return bool True if SMS in unknown status, otherwise - false |
||
220 | */ |
||
221 | View Code Duplication | public function smsInUnknownStatus($messageId) |
|
232 | |||
233 | // endregion |
||
234 | |||
235 | // region Private functions |
||
236 | |||
237 | /** |
||
238 | * Send request to check status of SMS |
||
239 | * |
||
240 | * @param int $messageId Message ID |
||
241 | * |
||
242 | * @return array|null |
||
243 | * |
||
244 | * @throws SinchPaymentRequiredException When run out of money |
||
245 | */ |
||
246 | private function sendRequestToCheckStatusOfSMS($messageId) |
||
272 | |||
273 | /** |
||
274 | * Create appropriate Sinch exception |
||
275 | * |
||
276 | * @param ClientException $e Exception |
||
277 | * |
||
278 | * @return \Exception|\Fresh\SinchBundle\Exception\SinchException |
||
279 | */ |
||
280 | private function createAppropriateSinchException(ClientException $e) |
||
314 | |||
315 | /** |
||
316 | * Get Sinch exception for bad request |
||
317 | * |
||
318 | * @param int $errorCode Sinch error code |
||
319 | * @param string $errorMessage Sinch error message |
||
320 | * |
||
321 | * @return \Fresh\SinchBundle\Exception\SinchException|null |
||
322 | */ |
||
323 | View Code Duplication | private function getSinchExceptionForBadRequest($errorCode, $errorMessage) |
|
341 | |||
342 | /** |
||
343 | * Get Sinch exception for unauthorized |
||
344 | * |
||
345 | * @param int $errorCode Sinch error code |
||
346 | * @param string $errorMessage Sinch error message |
||
347 | * |
||
348 | * @return \Fresh\SinchBundle\Exception\SinchException|null |
||
349 | */ |
||
350 | View Code Duplication | private function getSinchExceptionForUnauthorized($errorCode, $errorMessage) |
|
360 | |||
361 | /** |
||
362 | * Get Sinch exception for payment required |
||
363 | * |
||
364 | * Sinch returns 402 code when application run out of money |
||
365 | * |
||
366 | * @param int $errorCode Sinch error code |
||
367 | * @param string $errorMessage Sinch error message |
||
368 | * |
||
369 | * @return \Fresh\SinchBundle\Exception\SinchException|null |
||
370 | */ |
||
371 | View Code Duplication | private function getSinchExceptionForPaymentRequired($errorCode, $errorMessage) |
|
381 | |||
382 | /** |
||
383 | * Get Sinch exception for forbidden |
||
384 | * |
||
385 | * @param int $errorCode Sinch error code |
||
386 | * @param string $errorMessage Sinch error message |
||
387 | * |
||
388 | * @return \Fresh\SinchBundle\Exception\SinchException|null |
||
389 | */ |
||
390 | View Code Duplication | private function getSinchExceptionForForbidden($errorCode, $errorMessage) |
|
409 | |||
410 | /** |
||
411 | * Get Sinch exception for internal server error |
||
412 | * |
||
413 | * @param int $errorCode Sinch error code |
||
414 | * @param string $errorMessage Sinch error message |
||
415 | * |
||
416 | * @return \Fresh\SinchBundle\Exception\SinchException|null |
||
417 | */ |
||
418 | View Code Duplication | private function getSinchExceptionForInternalServerError($errorCode, $errorMessage) |
|
428 | |||
429 | // endregion |
||
430 | } |
||
431 |
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.