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 E2EHelper 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 E2EHelper, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
19 | class E2EHelper { |
||
20 | /** |
||
21 | * @var Connection |
||
22 | */ |
||
23 | private $connection; |
||
24 | |||
25 | /** |
||
26 | * @var CryptTool |
||
27 | */ |
||
28 | private $cryptTool; |
||
29 | |||
30 | /** |
||
31 | * @var string (bin) |
||
32 | */ |
||
33 | private $privateKey; |
||
34 | |||
35 | /** |
||
36 | * @param string $privateKey (binary) |
||
37 | * @param Connection $connection |
||
38 | * @param CryptTool $cryptTool |
||
39 | */ |
||
40 | public function __construct($privateKey, Connection $connection, CryptTool $cryptTool = null) { |
||
49 | |||
50 | /** |
||
51 | * Encrypt a text message and send it to the threemaId |
||
52 | * |
||
53 | * @param string $threemaId |
||
54 | * @param string $text |
||
55 | * @throws \Threema\Core\Exception |
||
56 | * @return \Threema\MsgApi\Commands\Results\SendE2EResult |
||
57 | */ |
||
58 | public final function sendTextMessage($threemaId, $text) { |
||
74 | |||
75 | /** |
||
76 | * Encrypt an image file, upload the blob and send the image message to the threemaId |
||
77 | * |
||
78 | * @param string $threemaId |
||
79 | * @param string $imagePath |
||
80 | * @return \Threema\MsgApi\Commands\Results\SendE2EResult |
||
81 | * @throws \Threema\Core\Exception |
||
82 | */ |
||
83 | public final function sendImageMessage($threemaId, $imagePath) { |
||
123 | |||
124 | /** |
||
125 | * Encrypt a file (and thumbnail if given), upload the blob and send it to the given threemaId |
||
126 | * |
||
127 | * @param string $threemaId |
||
128 | * @param string $filePath |
||
129 | * @param null|string $thumbnailPath |
||
130 | * @throws \Threema\Core\Exception |
||
131 | * @return \Threema\MsgApi\Commands\Results\SendE2EResult |
||
132 | */ |
||
133 | public final function sendFileMessage($threemaId, $filePath, $thumbnailPath = null) { |
||
181 | |||
182 | |||
183 | /** |
||
184 | * Decrypt a message and download the files of the message to the $outputFolder |
||
185 | * |
||
186 | * Note: This does not check the MAC before, which you should always do when |
||
187 | * you want to use this in your own application! Use {@link checkMac()} for doing so. |
||
188 | * |
||
189 | * @param string $threemaId |
||
190 | * @param string $messageId |
||
191 | * @param string $box box as binary string |
||
192 | * @param string $nonce nonce as binary string |
||
193 | * @param string|null|false $outputFolder folder for storing the files, |
||
194 | * null=current folder, false=do not download files |
||
195 | * @param \Closure $downloadMessage |
||
196 | * @return ReceiveMessageResult |
||
197 | * @throws Exception |
||
198 | * @throws \Threema\MsgApi\Exceptions\BadMessageException |
||
199 | * @throws \Threema\MsgApi\Exceptions\DecryptionFailedException |
||
200 | * @throws \Threema\MsgApi\Exceptions\UnsupportedMessageTypeException |
||
201 | */ |
||
202 | public final function receiveMessage($threemaId, |
||
298 | |||
299 | /** |
||
300 | * Check the HMAC of an ingoing Threema request. Always do this before de- |
||
301 | * crypting the message. |
||
302 | * |
||
303 | * @param string $threemaId |
||
304 | * @param string $gatewayId |
||
305 | * @param string $messageId |
||
306 | * @param string $date |
||
307 | * @param string $nonce nonce as hex encoded string |
||
308 | * @param string $box box as hex encoded string |
||
309 | * @param string $mac the original one send by the server |
||
310 | * |
||
311 | * @return bool true if check was successfull, false if not |
||
312 | */ |
||
313 | public final function checkMac($threemaId, $gatewayId, $messageId, $date, $nonce, $box, $mac, $secret) { |
||
317 | |||
318 | /** |
||
319 | * Fetch a public key and check the capability of the threemaId |
||
320 | * |
||
321 | * @param string $threemaId |
||
322 | * @param \Closure $capabilityCheck |
||
323 | * @return string Public key as binary |
||
324 | * @throws Exception |
||
325 | */ |
||
326 | private final function fetchPublicKeyAndCheckCapability($threemaId, \Closure $capabilityCheck = null) { |
||
344 | |||
345 | /** |
||
346 | * @param ThreemaMessage $message |
||
347 | * @param string $blobId blob id as hex |
||
348 | * @param \Closure|null $downloadMessage |
||
349 | * @return null|\Threema\MsgApi\Commands\Results\DownloadFileResult |
||
350 | * @throws Exception |
||
351 | */ |
||
352 | private final function downloadFile(ThreemaMessage $message, $blobId, \Closure $downloadMessage = null) { |
||
365 | } |
||
366 |
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.