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 ScriptCommand 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 ScriptCommand, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class ScriptCommand extends AbstractMagentoCommand |
||
17 | { |
||
18 | /** |
||
19 | * @var array |
||
20 | */ |
||
21 | protected $scriptVars = array(); |
||
22 | |||
23 | /** |
||
24 | * @var string |
||
25 | */ |
||
26 | protected $_scriptFilename = ''; |
||
27 | |||
28 | /** |
||
29 | * @var bool |
||
30 | */ |
||
31 | protected $_stopOnError = false; |
||
32 | |||
33 | View Code Duplication | protected function configure() |
|
|
|||
34 | { |
||
35 | $this |
||
36 | ->setName('script') |
||
37 | ->addArgument('filename', InputArgument::OPTIONAL, 'Script file') |
||
38 | ->addOption('define', 'd', InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, 'Defines a variable') |
||
39 | ->addOption('stop-on-error', null, InputOption::VALUE_NONE, 'Stops execution of script on error') |
||
40 | ->setDescription('Runs multiple n98-magerun commands') |
||
41 | ; |
||
42 | |||
43 | $help = <<<HELP |
||
44 | Example: |
||
45 | |||
46 | # Set multiple config |
||
47 | config:set "web/cookie/cookie_domain" example.com |
||
48 | |||
49 | # Set with multiline values with "\n" |
||
50 | config:set "general/store_information/address" "First line\nSecond line\nThird line" |
||
51 | |||
52 | # This is a comment |
||
53 | cache:flush |
||
54 | |||
55 | |||
56 | Optionally you can work with unix pipes. |
||
57 | |||
58 | \$ echo "cache:flush" | n98-magerun-dev script |
||
59 | |||
60 | \$ n98-magerun.phar script < filename |
||
61 | |||
62 | It is even possible to create executable scripts: |
||
63 | |||
64 | Create file `test.magerun` and make it executable (`chmod +x test.magerun`): |
||
65 | |||
66 | #!/usr/bin/env n98-magerun.phar script |
||
67 | |||
68 | config:set "web/cookie/cookie_domain" example.com |
||
69 | cache:flush |
||
70 | |||
71 | # Run a shell script with "!" as first char |
||
72 | ! ls -l |
||
73 | |||
74 | # Register your own variable (only key = value currently supported) |
||
75 | \${my.var}=bar |
||
76 | |||
77 | # Let magerun ask for variable value - add a question mark |
||
78 | \${my.var}=? |
||
79 | |||
80 | ! echo \${my.var} |
||
81 | |||
82 | # Use resolved variables from n98-magerun in shell commands |
||
83 | ! ls -l \${magento.root}/code/local |
||
84 | |||
85 | Pre-defined variables: |
||
86 | |||
87 | * \${magento.root} -> Magento Root-Folder |
||
88 | * \${magento.version} -> Magento Version i.e. 1.7.0.2 |
||
89 | * \${magento.edition} -> Magento Edition -> Community or Enterprise |
||
90 | * \${magerun.version} -> Magerun version i.e. 1.66.0 |
||
91 | * \${php.version} -> PHP Version |
||
92 | * \${script.file} -> Current script file path |
||
93 | * \${script.dir} -> Current script file dir |
||
94 | |||
95 | Variables can be passed to a script with "--define (-d)" option. |
||
96 | |||
97 | Example: |
||
98 | |||
99 | $ n98-magerun.phar script -d foo=bar filename |
||
100 | |||
101 | # This will register the variable \${foo} with value bar. |
||
102 | |||
103 | It's possible to define multiple values by passing more than one option. |
||
104 | HELP; |
||
105 | $this->setHelp($help); |
||
106 | } |
||
107 | |||
108 | /** |
||
109 | * @return bool |
||
110 | */ |
||
111 | public function isEnabled() |
||
112 | { |
||
113 | return Exec::allowed(); |
||
114 | } |
||
115 | |||
116 | protected function execute(InputInterface $input, OutputInterface $output) |
||
117 | { |
||
118 | $this->_scriptFilename = $input->getArgument('filename'); |
||
119 | $this->_stopOnError = $input->getOption('stop-on-error'); |
||
120 | $this->_initDefines($input); |
||
121 | $script = $this->_getContent($this->_scriptFilename); |
||
122 | $commands = explode("\n", $script); |
||
123 | $this->initScriptVars(); |
||
124 | |||
125 | foreach ($commands as $commandString) { |
||
126 | $commandString = trim($commandString); |
||
127 | if (empty($commandString)) { |
||
128 | continue; |
||
129 | } |
||
130 | $firstChar = substr($commandString, 0, 1); |
||
131 | |||
132 | switch ($firstChar) { |
||
133 | |||
134 | // comment |
||
135 | case '#': |
||
136 | continue; |
||
137 | break; |
||
138 | |||
139 | // set var |
||
140 | case '$': |
||
141 | $this->registerVariable($output, $commandString); |
||
142 | break; |
||
143 | |||
144 | // run shell script |
||
145 | case '!': |
||
146 | $this->runShellCommand($output, $commandString); |
||
147 | break; |
||
148 | |||
149 | default: |
||
150 | $this->runMagerunCommand($input, $output, $commandString); |
||
151 | } |
||
152 | } |
||
153 | } |
||
154 | |||
155 | /** |
||
156 | * @param InputInterface $input |
||
157 | * @throws InvalidArgumentException |
||
158 | */ |
||
159 | protected function _initDefines(InputInterface $input) |
||
160 | { |
||
161 | $defines = $input->getOption('define'); |
||
162 | if (is_string($defines)) { |
||
163 | $defines = array($defines); |
||
164 | } |
||
165 | if (count($defines) > 0) { |
||
166 | foreach ($defines as $define) { |
||
167 | if (!strstr($define, '=')) { |
||
168 | throw new InvalidArgumentException('Invalid define'); |
||
169 | } |
||
170 | $parts = BinaryString::trimExplodeEmpty('=', $define); |
||
171 | $variable = $parts[0]; |
||
172 | $value = null; |
||
173 | if (isset($parts[1])) { |
||
174 | $value = $parts[1]; |
||
175 | } |
||
176 | $this->scriptVars['${' . $variable . '}'] = $value; |
||
177 | } |
||
178 | } |
||
179 | } |
||
180 | |||
181 | /** |
||
182 | * @param string $filename |
||
183 | * @throws RuntimeException |
||
184 | * @internal param string $input |
||
185 | * @return string |
||
186 | */ |
||
187 | protected function _getContent($filename) |
||
188 | { |
||
189 | if ($filename == '-' || empty($filename)) { |
||
190 | $script = @\file_get_contents('php://stdin', 'r'); |
||
191 | } else { |
||
192 | $script = @\file_get_contents($filename); |
||
193 | } |
||
194 | |||
195 | if (!$script) { |
||
196 | throw new RuntimeException('Script file was not found'); |
||
197 | } |
||
198 | |||
199 | return $script; |
||
200 | } |
||
201 | |||
202 | /** |
||
203 | * @param OutputInterface $output |
||
204 | * @param string $commandString |
||
205 | * @throws RuntimeException |
||
206 | * @return void |
||
207 | */ |
||
208 | protected function registerVariable(OutputInterface $output, $commandString) |
||
255 | |||
256 | /** |
||
257 | * @param InputInterface $input |
||
258 | * @param OutputInterface $output |
||
259 | * @param string $commandString |
||
260 | * @throws RuntimeException |
||
261 | */ |
||
262 | protected function runMagerunCommand(InputInterface $input, OutputInterface $output, $commandString) |
||
263 | { |
||
264 | $this->getApplication()->setAutoExit(false); |
||
265 | $commandString = $this->_replaceScriptVars($commandString); |
||
266 | $input = new StringInput($commandString); |
||
267 | $exitCode = $this->getApplication()->run($input, $output); |
||
268 | if ($exitCode !== 0 && $this->_stopOnError) { |
||
269 | throw new RuntimeException('Script stopped with errors'); |
||
270 | } |
||
271 | } |
||
272 | |||
273 | /** |
||
274 | * @param string $commandString |
||
275 | * @return string |
||
276 | */ |
||
277 | protected function _prepareShellCommand($commandString) |
||
278 | { |
||
279 | $commandString = ltrim($commandString, '!'); |
||
280 | |||
281 | // @TODO find a better place |
||
282 | if (strstr($commandString, '${magento.root}') |
||
283 | || strstr($commandString, '${magento.version}') |
||
284 | || strstr($commandString, '${magento.edition}') |
||
285 | ) { |
||
286 | $this->initMagento(); |
||
287 | } |
||
288 | $this->initScriptVars(); |
||
289 | $commandString = $this->_replaceScriptVars($commandString); |
||
290 | |||
291 | return $commandString; |
||
292 | } |
||
293 | |||
294 | protected function initScriptVars() |
||
308 | |||
309 | /** |
||
310 | * @param OutputInterface $output |
||
311 | * @param string $commandString |
||
312 | * @internal param $returnValue |
||
313 | */ |
||
314 | protected function runShellCommand(OutputInterface $output, $commandString) |
||
315 | { |
||
322 | |||
323 | /** |
||
324 | * @param string $commandString |
||
325 | * |
||
326 | * @return string |
||
327 | */ |
||
328 | protected function _replaceScriptVars($commandString) |
||
334 | } |
||
335 |
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.