Complex classes like PackageManager 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 PackageManager, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
22 | abstract class PackageManager |
||
23 | { |
||
24 | /** |
||
25 | * @var Plugin the plugin instance |
||
26 | */ |
||
27 | protected $plugin; |
||
28 | |||
29 | /** |
||
30 | * @var string Package manager name: `bower` or `npm` |
||
31 | */ |
||
32 | protected $name; |
||
33 | |||
34 | /** |
||
35 | * @var string Package config file name: `bower.json` or `package.json` |
||
36 | */ |
||
37 | public $file; |
||
38 | |||
39 | /** |
||
40 | * @var string Path to package manager binary |
||
41 | */ |
||
42 | public $bin; |
||
43 | |||
44 | /** |
||
45 | * @var string Package name of the PHP version of the package manager |
||
46 | */ |
||
47 | public $phpPackage; |
||
48 | |||
49 | /** |
||
50 | * @var string Binary name of PHP version of package manager |
||
51 | */ |
||
52 | protected $phpBin; |
||
53 | |||
54 | /** |
||
55 | * @var array Package config. Initially holds default config |
||
56 | */ |
||
57 | protected $config = []; |
||
58 | |||
59 | /** |
||
60 | * @var array List of keys holding dependencies |
||
61 | */ |
||
62 | protected $dependencies = ['dependencies', 'devDependencies']; |
||
63 | |||
64 | /** |
||
65 | * Reads config file or dist config if exists, merges with default config. |
||
66 | * @param Plugin $plugin |
||
67 | * @void |
||
68 | */ |
||
69 | 3 | public function __construct(Plugin $plugin) |
|
79 | |||
80 | /** |
||
81 | * Reads the JSON config from the $path. |
||
82 | * |
||
83 | * @param string $path path to the Json file |
||
84 | * @return array|mixed |
||
85 | */ |
||
86 | 3 | public function readConfig($path) |
|
97 | |||
98 | /** |
||
99 | * Saves JSON config to the given path. |
||
100 | * |
||
101 | * @param string $path |
||
102 | * @param array $config |
||
103 | * @throws \Exception |
||
104 | */ |
||
105 | public function writeConfig($path, array $config) |
||
106 | { |
||
107 | foreach ($this->dependencies as $key) { |
||
108 | if (isset($config[$key]) && !$config[$key]) { |
||
109 | unset($config[$key]); |
||
110 | } |
||
111 | } |
||
112 | $jsonFile = new JsonFile($path); |
||
113 | $jsonFile->write($config); |
||
114 | } |
||
115 | |||
116 | /** |
||
117 | * Scans the $package and extracts dependencies to the [[config]]. |
||
118 | * |
||
119 | * @param CompletePackageInterface $package |
||
120 | * @see mergeConfig() |
||
121 | * @void |
||
122 | */ |
||
123 | public function scanPackage(CompletePackageInterface $package) |
||
124 | { |
||
125 | $extra = $package->getExtra(); |
||
126 | $config = []; |
||
127 | foreach ($this->dependencies as $key) { |
||
128 | $name = $this->name . '-' . $key; |
||
129 | if (isset($extra[$name])) { |
||
130 | $config[$key] = $extra[$name]; |
||
131 | } |
||
132 | } |
||
133 | if (!empty($config)) { |
||
134 | $this->mergeConfig($config); |
||
135 | } |
||
136 | } |
||
137 | |||
138 | /** |
||
139 | * Merges the $config over the [[config]], doesn't resolve version conflicts. |
||
140 | * @param array $config |
||
141 | * @see mergeVersions() |
||
142 | * @void |
||
143 | */ |
||
144 | protected function mergeConfig(array $config) |
||
156 | |||
157 | /** |
||
158 | * @param $a |
||
159 | * @param $b |
||
160 | * @return string |
||
161 | */ |
||
162 | protected function mergeVersions($a, $b) |
||
175 | |||
176 | /** |
||
177 | * Check if $a is more then $b, like: a="1.1 || 2.2" b="1.1" |
||
178 | * Possible optimization. |
||
179 | * // TODO Rename and implement. |
||
180 | * @param string $a |
||
181 | * @param string $b |
||
182 | * @return boolean |
||
183 | */ |
||
184 | public function isMoreVersion($a, $b) |
||
188 | |||
189 | /** |
||
190 | * Checks whether the $version represents any possible version. |
||
191 | * |
||
192 | * @param string $version |
||
193 | * @return boolean |
||
194 | */ |
||
195 | public function isAnyVersion($version) |
||
199 | |||
200 | /** |
||
201 | * Set config. |
||
202 | * @param array $config |
||
203 | */ |
||
204 | public function setConfig(array $config) |
||
208 | |||
209 | /** |
||
210 | * Returns if the package manager has nonempty dependency list. |
||
211 | * @return bool |
||
212 | */ |
||
213 | 1 | public function hasDependencies() |
|
223 | |||
224 | /** |
||
225 | * Run the given action: show notice, write config and run `perform`. |
||
226 | * @param string $action the action name |
||
227 | * @void |
||
228 | */ |
||
229 | public function runAction($action) |
||
236 | |||
237 | /** |
||
238 | * Run installation. Specific for every package manager. |
||
239 | * @param string $action the action name |
||
240 | * @void |
||
241 | */ |
||
242 | protected function perform($action) |
||
248 | |||
249 | /** |
||
250 | * Prepares arguments and runs the command with [[passthru()]]. |
||
251 | * @param array $arguments |
||
252 | * @return integer the exit code |
||
253 | */ |
||
254 | public function passthru(array $arguments = []) |
||
259 | |||
260 | /** |
||
261 | * Prepares given command arguments. |
||
262 | * @param array $arguments |
||
263 | * @return string |
||
264 | */ |
||
265 | public function prepareCommand(array $arguments = []) |
||
274 | |||
275 | /** |
||
276 | * Set path to binary executable file. |
||
277 | * @param $bin |
||
278 | * @internal param string $value |
||
279 | */ |
||
280 | public function setBin($bin) |
||
284 | |||
285 | /** |
||
286 | * Get path to the binary executable file. |
||
287 | * @return string |
||
288 | */ |
||
289 | public function getBin() |
||
297 | |||
298 | /** |
||
299 | * Find path to the binary. |
||
300 | * @return string |
||
301 | */ |
||
302 | public function detectBin() |
||
310 | } |
||
311 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.