Complex classes like MediawikiApi 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 MediawikiApi, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
25 | class MediawikiApi implements LoggerAwareInterface { |
||
26 | |||
27 | /** |
||
28 | * @var Client|null Should be accessed through getClient |
||
29 | */ |
||
30 | private $client = null; |
||
31 | |||
32 | /** |
||
33 | * @var bool|string |
||
34 | */ |
||
35 | private $isLoggedIn; |
||
36 | |||
37 | /** |
||
38 | * @var MediawikiSession |
||
39 | */ |
||
40 | private $session; |
||
41 | |||
42 | /** |
||
43 | * @var string |
||
44 | */ |
||
45 | private $version; |
||
46 | |||
47 | /** |
||
48 | * @var LoggerInterface |
||
49 | */ |
||
50 | private $logger; |
||
51 | |||
52 | /** |
||
53 | * @param string $apiUrl The API Url |
||
54 | * @param Client|null $client Guzzle Client |
||
55 | * @param MediawikiSession|null $session Inject a custom session here |
||
56 | */ |
||
57 | 23 | public function __construct( $apiUrl, Client $client = null, MediawikiSession $session = null ) { |
|
77 | |||
78 | /** |
||
79 | * @return Client |
||
80 | */ |
||
81 | 16 | private function getClient() { |
|
96 | |||
97 | /** |
||
98 | * Sets a logger instance on the object |
||
99 | * |
||
100 | * @since 1.1 |
||
101 | * |
||
102 | * @param LoggerInterface $logger |
||
103 | * |
||
104 | * @return null |
||
105 | */ |
||
106 | public function setLogger( LoggerInterface $logger ) { |
||
110 | |||
111 | /** |
||
112 | * @since 2.0 |
||
113 | * |
||
114 | * @param Request $request |
||
115 | * |
||
116 | * @return PromiseInterface |
||
117 | * Normally promising an array, though can be mixed (json_decode result) |
||
118 | * Can throw UsageExceptions or RejectionExceptions |
||
119 | */ |
||
120 | public function getRequestAsync( Request $request ) { |
||
130 | |||
131 | /** |
||
132 | * @since 2.0 |
||
133 | * |
||
134 | * @param Request $request |
||
135 | * |
||
136 | * @return PromiseInterface |
||
137 | * Normally promising an array, though can be mixed (json_decode result) |
||
138 | * Can throw UsageExceptions or RejectionExceptions |
||
139 | */ |
||
140 | public function postRequestAsync( Request $request ) { |
||
150 | |||
151 | /** |
||
152 | * @since 0.2 |
||
153 | * |
||
154 | * @param Request $request |
||
155 | * |
||
156 | * @return mixed Normally an array |
||
157 | */ |
||
158 | 8 | public function getRequest( Request $request ) { |
|
166 | |||
167 | /** |
||
168 | * @since 0.2 |
||
169 | * |
||
170 | * @param Request $request |
||
171 | * |
||
172 | * @return mixed Normally an array |
||
173 | */ |
||
174 | 8 | public function postRequest( Request $request ) { |
|
182 | |||
183 | /** |
||
184 | * @param ResponseInterface $response |
||
185 | * |
||
186 | * @return mixed |
||
187 | * @throws UsageException |
||
188 | */ |
||
189 | 16 | private function decodeResponse( ResponseInterface $response ) { |
|
197 | |||
198 | /** |
||
199 | * @param Request $request |
||
200 | * @param string $paramsKey either 'query' or 'form_params' |
||
201 | * |
||
202 | * @throws RequestException |
||
203 | * |
||
204 | * @return array as needed by ClientInterface::get and ClientInterface::post |
||
205 | */ |
||
206 | 16 | private function getClientRequestOptions( Request $request, $paramsKey ) { |
|
212 | |||
213 | /** |
||
214 | * @return array |
||
215 | */ |
||
216 | 16 | private function getDefaultHeaders() { |
|
221 | |||
222 | 16 | private function getUserAgent() { |
|
229 | |||
230 | /** |
||
231 | * @param $result |
||
232 | */ |
||
233 | 16 | private function logWarnings( $result ) { |
|
240 | |||
241 | /** |
||
242 | * @param array $result |
||
243 | * |
||
244 | * @throws UsageException |
||
245 | */ |
||
246 | 16 | private function throwUsageExceptions( $result ) { |
|
255 | |||
256 | /** |
||
257 | * @since 0.1 |
||
258 | * |
||
259 | * @return bool|string false or the name of the current user |
||
260 | */ |
||
261 | 16 | public function isLoggedin() { |
|
264 | |||
265 | /** |
||
266 | * @since 0.1 |
||
267 | * |
||
268 | * @param ApiUser $apiUser |
||
269 | * |
||
270 | * @throws UsageException |
||
271 | * @return bool success |
||
272 | */ |
||
273 | 2 | public function login( ApiUser $apiUser ) { |
|
298 | |||
299 | /** |
||
300 | * @param array $result |
||
301 | * |
||
302 | * @throws UsageException |
||
303 | */ |
||
304 | 1 | private function throwLoginUsageException( $result ) { |
|
363 | |||
364 | /** |
||
365 | * @since 0.1 |
||
366 | * @return bool success |
||
367 | */ |
||
368 | 2 | public function logout() { |
|
378 | |||
379 | /** |
||
380 | * @since 0.1 |
||
381 | * |
||
382 | * @param string $type |
||
383 | * |
||
384 | * @return string |
||
385 | */ |
||
386 | public function getToken( $type = 'csrf' ) { |
||
389 | |||
390 | /** |
||
391 | * @since 0.1 |
||
392 | * Clears all tokens stored by the api |
||
393 | */ |
||
394 | 1 | public function clearTokens() { |
|
397 | |||
398 | /** |
||
399 | * @return string |
||
400 | */ |
||
401 | 4 | public function getVersion(){ |
|
416 | |||
417 | } |
||
418 |
This check looks for the bodies of
if
statements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete.These
if
bodies can be removed. If you have an empty if but statements in theelse
branch, consider inverting the condition.could be turned into
This is much more concise to read.