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 OAuthClient 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 OAuthClient, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
36 | class OAuthClient |
||
37 | { |
||
38 | /** @var TokenStorageInterface */ |
||
39 | private $tokenStorage; |
||
40 | |||
41 | /** @var \fkooman\OAuth\Client\Http\HttpClientInterface */ |
||
42 | private $httpClient; |
||
43 | |||
44 | /** @var SessionInterface */ |
||
45 | private $session; |
||
46 | |||
47 | /** @var RandomInterface */ |
||
48 | private $random; |
||
49 | |||
50 | /** @var \DateTime */ |
||
51 | private $dateTime; |
||
52 | |||
53 | /** @var Provider */ |
||
54 | private $provider = null; |
||
55 | |||
56 | /** @var string */ |
||
57 | private $userId = null; |
||
58 | |||
59 | /** |
||
60 | * @param TokenStorageInterface $tokenStorage |
||
61 | * @param Http\HttpClientInterface $httpClient |
||
62 | * @param SessionInterface|null $session |
||
63 | * @param RandomInterface|null $random |
||
64 | * @param \DateTime|null $dateTime |
||
65 | */ |
||
66 | public function __construct( |
||
67 | TokenStorageInterface $tokenStorage, |
||
68 | HttpClientInterface $httpClient, |
||
69 | SessionInterface $session = null, |
||
70 | RandomInterface $random = null, |
||
71 | DateTime $dateTime = null |
||
72 | ) { |
||
73 | $this->tokenStorage = $tokenStorage; |
||
74 | $this->httpClient = $httpClient; |
||
75 | if (is_null($session)) { |
||
76 | $session = new Session(); |
||
77 | } |
||
78 | $this->session = $session; |
||
79 | if (is_null($random)) { |
||
80 | $random = new Random(); |
||
81 | } |
||
82 | $this->random = $random; |
||
83 | if (is_null($dateTime)) { |
||
84 | $dateTime = new DateTime(); |
||
85 | } |
||
86 | $this->dateTime = $dateTime; |
||
87 | } |
||
88 | |||
89 | /** |
||
90 | * @param \DateTime $dateTime |
||
91 | */ |
||
92 | public function setDateTime(DateTime $dateTime) |
||
93 | { |
||
94 | $this->dateTime = $dateTime; |
||
95 | } |
||
96 | |||
97 | /** |
||
98 | * @param Provider $provider |
||
99 | */ |
||
100 | public function setProvider(Provider $provider) |
||
101 | { |
||
102 | $this->provider = $provider; |
||
103 | } |
||
104 | |||
105 | /** |
||
106 | * @param string $userId |
||
107 | */ |
||
108 | public function setUserId($userId) |
||
112 | |||
113 | /** |
||
114 | * Perform a GET request, convenience wrapper for ::send(). |
||
115 | * |
||
116 | * @param string $requestScope |
||
117 | * @param string $requestUri |
||
118 | * @param array $requestHeaders |
||
119 | * |
||
120 | * @return Http\Response|false |
||
121 | */ |
||
122 | public function get($requestScope, $requestUri, array $requestHeaders = []) |
||
126 | |||
127 | /** |
||
128 | * Perform a POST request, convenience wrapper for ::send(). |
||
129 | * |
||
130 | * @param string $requestScope |
||
131 | * @param string $requestUri |
||
132 | * @param array $postBody |
||
133 | * @param array $requestHeaders |
||
134 | * |
||
135 | * @return Http\Response|false |
||
136 | */ |
||
137 | public function post($requestScope, $requestUri, array $postBody, array $requestHeaders = []) |
||
141 | |||
142 | /** |
||
143 | * Perform a HTTP request. |
||
144 | * |
||
145 | * @param string $requestScope |
||
146 | * @param Http\Request $request |
||
147 | * |
||
148 | * @return Response|false |
||
149 | */ |
||
150 | public function send($requestScope, Request $request) |
||
191 | |||
192 | /** |
||
193 | * Obtain an authorization request URL to start the authorization process |
||
194 | * at the OAuth provider. |
||
195 | * |
||
196 | * @param string $scope the space separated scope tokens |
||
197 | * @param string $redirectUri the URL registered at the OAuth provider, to |
||
198 | * be redirected back to |
||
199 | * |
||
200 | * @return string the authorization request URL |
||
201 | * |
||
202 | * @see https://tools.ietf.org/html/rfc6749#section-3.3 |
||
203 | * @see https://tools.ietf.org/html/rfc6749#section-3.1.2 |
||
204 | */ |
||
205 | public function getAuthorizeUri($scope, $redirectUri) |
||
243 | |||
244 | /** |
||
245 | * @param string $responseCode the code passed to the "code" |
||
246 | * query parameter on the callback URL |
||
247 | * @param string $responseState the state passed to the "state" |
||
248 | * query parameter on the callback URL |
||
249 | */ |
||
250 | public function handleCallback($responseCode, $responseState) |
||
333 | |||
334 | /** |
||
335 | * Verify if an AccessToken in the list that matches this scope, bound to |
||
336 | * providerId and userId. |
||
337 | * |
||
338 | * This method has NO side effects, i.e. it will not try to use, refresh or |
||
339 | * delete AccessTokens. If a token is expired, but a refresh token is |
||
340 | * available it is assumed that an AccessToken is available. |
||
341 | * |
||
342 | * NOTE: this does not mean that the token will also be accepted by the |
||
343 | * resource server! |
||
344 | * |
||
345 | * @param string $scope |
||
346 | * |
||
347 | * @return bool |
||
348 | */ |
||
349 | public function hasAccessToken($scope) |
||
370 | |||
371 | /** |
||
372 | * @param AccessToken $accessToken |
||
373 | * |
||
374 | * @return AccessToken|false |
||
375 | */ |
||
376 | private function refreshAccessToken(AccessToken $accessToken) |
||
439 | |||
440 | /** |
||
441 | * Find an AccessToken in the list that matches this scope, bound to |
||
442 | * providerId and userId. |
||
443 | * |
||
444 | * @param string $scope |
||
445 | * |
||
446 | * @return AccessToken|false |
||
447 | */ |
||
448 | private function getAccessToken($scope) |
||
464 | |||
465 | /** |
||
466 | * @param string $codeVerifier |
||
467 | * |
||
468 | * @return string |
||
469 | */ |
||
470 | private static function hashCodeVerifier($codeVerifier) |
||
483 | |||
484 | /** |
||
485 | * @return string |
||
486 | */ |
||
487 | private function generateCodeVerifier() |
||
496 | } |
||
497 |
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.