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 XoopsBlock 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 XoopsBlock, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
30 | class XoopsBlock extends XoopsObject |
||
31 | { |
||
32 | const CUSTOM_HTML = 'H'; // custom HTML block |
||
33 | const CUSTOM_PHP = 'P'; // custom PHP block |
||
34 | const CUSTOM_SMILIE = 'S'; // use text sanitizer (smilies enabled) |
||
35 | const CUSTOM_TEXT = 'T'; // use text sanitizer (smilies disabled) |
||
36 | |||
37 | const BLOCK_TYPE_SYSTEM = 'S'; // S - generated by system module |
||
38 | const BLOCK_TYPE_MODULE = 'M'; // M - generated by a non-system module |
||
39 | const BLOCK_TYPE_CUSTOM = 'C'; // C - Custom block |
||
40 | const BLOCK_TYPE_CLONED = 'D'; // D - cloned system/module block |
||
41 | // E - cloned custom block, DON'T use it |
||
42 | |||
43 | /** |
||
44 | * Constructor |
||
45 | * |
||
46 | * @param int|array $id object id |
||
47 | */ |
||
48 | 34 | public function __construct($id = null) |
|
49 | { |
||
50 | 34 | $this->initVar('bid', Dtype::TYPE_INTEGER, null, false); |
|
51 | 34 | $this->initVar('mid', Dtype::TYPE_INTEGER, 0, false); |
|
52 | 34 | $this->initVar('func_num', Dtype::TYPE_INTEGER, 0, false); |
|
53 | 34 | $this->initVar('options', Dtype::TYPE_TEXT_BOX, null, false, 255); |
|
54 | 34 | $this->initVar('name', Dtype::TYPE_TEXT_BOX, null, true, 150); |
|
55 | //$this->initVar('position', Dtype::TYPE_INTEGER, 0, false); |
||
56 | 34 | $this->initVar('title', Dtype::TYPE_TEXT_BOX, null, false, 150); |
|
57 | 34 | $this->initVar('content', Dtype::TYPE_TEXT_AREA, null, false); |
|
58 | 34 | $this->initVar('side', Dtype::TYPE_INTEGER, 0, false); |
|
59 | 34 | $this->initVar('weight', Dtype::TYPE_INTEGER, 0, false); |
|
60 | 34 | $this->initVar('visible', Dtype::TYPE_INTEGER, 0, false); |
|
61 | 34 | $this->initVar('block_type', Dtype::TYPE_OTHER, null, false); |
|
62 | 34 | $this->initVar('c_type', Dtype::TYPE_OTHER, null, false); |
|
63 | 34 | $this->initVar('isactive', Dtype::TYPE_INTEGER, null, false); |
|
64 | 34 | $this->initVar('dirname', Dtype::TYPE_TEXT_BOX, null, false, 50); |
|
65 | 34 | $this->initVar('func_file', Dtype::TYPE_TEXT_BOX, null, false, 50); |
|
66 | 34 | $this->initVar('show_func', Dtype::TYPE_TEXT_BOX, null, false, 50); |
|
67 | 34 | $this->initVar('edit_func', Dtype::TYPE_TEXT_BOX, null, false, 50); |
|
68 | 34 | $this->initVar('template', Dtype::TYPE_OTHER, null, false); |
|
69 | 34 | $this->initVar('bcachetime', Dtype::TYPE_INTEGER, 0, false); |
|
70 | 34 | $this->initVar('last_modified', Dtype::TYPE_INTEGER, 0, false); |
|
71 | |||
72 | 34 | $xoops = \Xoops::getInstance(); |
|
73 | |||
74 | // for backward compatibility |
||
75 | 34 | if (isset($id)) { |
|
76 | 6 | if (is_array($id)) { |
|
77 | 5 | $this->assignVars($id); |
|
78 | } else { |
||
79 | 1 | $blockHandler = $xoops->getHandlerBlock(); |
|
80 | 1 | $obj = $blockHandler->get($id); |
|
81 | 1 | foreach (array_keys($obj->getVars()) as $i) { |
|
82 | 1 | $this->assignVar($i, $obj->getVar($i, 'n')); |
|
83 | } |
||
84 | } |
||
85 | } |
||
86 | 34 | } |
|
87 | |||
88 | |||
89 | /** |
||
90 | * getter |
||
91 | * |
||
92 | * @param string $format Dtype::FORMAT_xxxx constant |
||
93 | * |
||
94 | * @return mixed |
||
95 | */ |
||
96 | 1 | public function id($format = 'n') |
|
100 | |||
101 | /** |
||
102 | * getter |
||
103 | * |
||
104 | * @param string $format Dtype::FORMAT_xxxx constant |
||
105 | * |
||
106 | * @return mixed |
||
107 | */ |
||
108 | 2 | public function bid($format = '') |
|
112 | |||
113 | /** |
||
114 | * getter |
||
115 | * |
||
116 | * @param string $format Dtype::FORMAT_xxxx constant |
||
117 | * |
||
118 | * @return mixed |
||
119 | */ |
||
120 | 1 | public function mid($format = '') |
|
124 | |||
125 | /** |
||
126 | * getter |
||
127 | * |
||
128 | * @param string $format Dtype::FORMAT_xxxx constant |
||
129 | * |
||
130 | * @return mixed |
||
131 | */ |
||
132 | 1 | public function func_num($format = '') |
|
136 | |||
137 | /** |
||
138 | * getter |
||
139 | * |
||
140 | * @param string $format Dtype::FORMAT_xxxx constant |
||
141 | * |
||
142 | * @return mixed |
||
143 | */ |
||
144 | 1 | public function options($format = '') |
|
148 | |||
149 | /** |
||
150 | * getter |
||
151 | * |
||
152 | * @param string $format Dtype::FORMAT_xxxx constant |
||
153 | * |
||
154 | * @return mixed |
||
155 | */ |
||
156 | 1 | public function name($format = '') |
|
160 | |||
161 | /** |
||
162 | * getter |
||
163 | * |
||
164 | * @param string $format Dtype::FORMAT_xxxx constant |
||
165 | * |
||
166 | * @return mixed |
||
167 | */ |
||
168 | 1 | public function title($format = '') |
|
172 | |||
173 | /** |
||
174 | * getter |
||
175 | * |
||
176 | * @param string $format Dtype::FORMAT_xxxx constant |
||
177 | * |
||
178 | * @return mixed |
||
179 | */ |
||
180 | 1 | public function content($format = '') |
|
184 | |||
185 | /** |
||
186 | * getter |
||
187 | * |
||
188 | * @param string $format Dtype::FORMAT_xxxx constant |
||
189 | * |
||
190 | * @return mixed |
||
191 | */ |
||
192 | 1 | public function side($format = '') |
|
196 | |||
197 | /** |
||
198 | * getter |
||
199 | * |
||
200 | * @param string $format Dtype::FORMAT_xxxx constant |
||
201 | * |
||
202 | * @return mixed |
||
203 | */ |
||
204 | 1 | public function weight($format = '') |
|
208 | |||
209 | /** |
||
210 | * getter |
||
211 | * |
||
212 | * @param string $format Dtype::FORMAT_xxxx constant |
||
213 | * |
||
214 | * @return mixed |
||
215 | */ |
||
216 | 1 | public function visible($format = '') |
|
220 | |||
221 | /** |
||
222 | * getter |
||
223 | * |
||
224 | * @param string $format Dtype::FORMAT_xxxx constant |
||
225 | * |
||
226 | * @return mixed |
||
227 | */ |
||
228 | 1 | public function block_type($format = '') |
|
229 | { |
||
230 | 1 | return $this->getVar('block_type', $format); |
|
231 | } |
||
232 | |||
233 | /** |
||
234 | * getter custom block type XoopsBlock::CUSTOM_xxxx constant |
||
235 | * |
||
236 | * @param string $format Dtype::FORMAT_xxxx constant |
||
237 | * |
||
238 | * @return mixed |
||
239 | */ |
||
240 | 1 | public function c_type($format = '') |
|
244 | |||
245 | /** |
||
246 | * getter |
||
247 | * |
||
248 | * @param string $format Dtype::FORMAT_xxxx constant |
||
249 | * |
||
250 | * @return mixed |
||
251 | */ |
||
252 | 1 | public function isactive($format = '') |
|
256 | |||
257 | /** |
||
258 | * getter |
||
259 | * |
||
260 | * @param string $format Dtype::FORMAT_xxxx constant |
||
261 | * |
||
262 | * @return mixed |
||
263 | */ |
||
264 | 1 | public function dirname($format = '') |
|
268 | |||
269 | /** |
||
270 | * getter |
||
271 | * |
||
272 | * @param string $format Dtype::FORMAT_xxxx constant |
||
273 | * |
||
274 | * @return mixed |
||
275 | */ |
||
276 | 1 | public function func_file($format = '') |
|
280 | |||
281 | /** |
||
282 | * getter |
||
283 | * |
||
284 | * @param string $format Dtype::FORMAT_xxxx constant |
||
285 | * |
||
286 | * @return mixed |
||
287 | */ |
||
288 | 1 | public function show_func($format = '') |
|
292 | |||
293 | /** |
||
294 | * getter |
||
295 | * |
||
296 | * @param string $format Dtype::FORMAT_xxxx constant |
||
297 | * |
||
298 | * @return mixed |
||
299 | */ |
||
300 | 1 | public function edit_func($format = '') |
|
304 | |||
305 | /** |
||
306 | * getter |
||
307 | * |
||
308 | * @param string $format Dtype::FORMAT_xxxx constant |
||
309 | * |
||
310 | * @return mixed |
||
311 | */ |
||
312 | 1 | public function template($format = '') |
|
316 | |||
317 | /** |
||
318 | * getter |
||
319 | * |
||
320 | * @param string $format Dtype::FORMAT_xxxx constant |
||
321 | * |
||
322 | * @return mixed |
||
323 | */ |
||
324 | 1 | public function bcachetime($format = '') |
|
328 | |||
329 | /** |
||
330 | * getter |
||
331 | * |
||
332 | * @param string $format Dtype::FORMAT_xxxx constant |
||
333 | * |
||
334 | * @return mixed |
||
335 | */ |
||
336 | 1 | public function last_modified($format = '') |
|
337 | { |
||
338 | 1 | return $this->getVar('last_modified', $format); |
|
339 | } |
||
340 | |||
341 | /** |
||
342 | * return the content of the block for output |
||
343 | * |
||
344 | * @param string $format Dtype::FORMAT_xxxx constant |
||
345 | * @param string $c_type type of custom content, a XoopsBlock::CUSTOM_xxxx constant |
||
346 | * H : custom HTML block |
||
347 | * P : custom PHP block |
||
348 | * S : use text sanitizer (smilies enabled) |
||
349 | * T : use text sanitizer (smilies disabled) |
||
350 | * |
||
351 | * @return string content for output |
||
352 | */ |
||
353 | public function getContent($format = 's', $c_type = 'T') |
||
354 | { |
||
355 | $format = strtolower($format); |
||
356 | $c_type = strtoupper($c_type); |
||
357 | switch ($format) { |
||
358 | case Dtype::FORMAT_SHOW: |
||
359 | case 's': |
||
360 | // apply c_type rules for content display |
||
361 | $content = $this->getVar('content', Dtype::FORMAT_NONE); |
||
362 | switch ($c_type) { |
||
363 | case XoopsBlock::CUSTOM_HTML: |
||
364 | return $this->convertSiteURL($content); |
||
365 | case XoopsBlock::CUSTOM_PHP: |
||
366 | ob_start(); |
||
367 | echo eval($content); |
||
|
|||
368 | $content = ob_get_contents(); |
||
369 | ob_end_clean(); |
||
370 | return $this->convertSiteURL($content); |
||
371 | case XoopsBlock::CUSTOM_SMILIE: |
||
372 | $myts = Sanitizer::getInstance(); |
||
373 | return $myts->filterForDisplay($this->convertSiteURL($content), 1, 1); |
||
374 | case XoopsBlock::CUSTOM_TEXT: |
||
375 | default: |
||
376 | $myts = Sanitizer::getInstance(); |
||
377 | return $myts->filterForDisplay($this->convertSiteURL($content), 1, 0); |
||
378 | } |
||
379 | break; |
||
380 | case Dtype::FORMAT_EDIT: |
||
381 | case 'e': |
||
382 | return $this->getVar('content', Dtype::FORMAT_EDIT); |
||
383 | break; |
||
384 | default: |
||
385 | return $this->getVar('content', Dtype::FORMAT_NONE); |
||
386 | break; |
||
387 | } |
||
388 | } |
||
389 | |||
390 | /** |
||
391 | * Convert {X_SITEURL} to actual site URL in content |
||
392 | * |
||
393 | * @param string $content content to convert |
||
394 | * |
||
395 | * @return string |
||
396 | */ |
||
397 | protected function convertSiteURL($content) |
||
398 | { |
||
399 | $content = str_replace('{X_SITEURL}', \Xoops::getInstance()->url('/'), $content); |
||
400 | return $content; |
||
401 | } |
||
402 | |||
403 | /** |
||
404 | * (HTML-) form for setting the options of the block |
||
405 | * |
||
406 | * @return string|false HTML for the form, FALSE if not defined for this block |
||
407 | */ |
||
408 | 1 | public function getOptions() |
|
409 | { |
||
410 | 1 | $xoops = \Xoops::getInstance(); |
|
411 | 1 | if (!$this->isCustom()) { |
|
412 | 1 | $edit_func = (string)$this->getVar('edit_func'); |
|
413 | 1 | if (!$edit_func) { |
|
414 | 1 | return false; |
|
415 | } |
||
416 | 1 | $funcFile = $xoops->path( |
|
417 | 1 | 'modules/' . $this->getVar('dirname') . '/blocks/' . $this->getVar('func_file') |
|
418 | ); |
||
419 | 1 | if (\XoopsLoad::fileExists($funcFile)) { |
|
420 | 1 | $xoops->loadLanguage('blocks', $this->getVar('dirname')); |
|
421 | 1 | include_once $funcFile; |
|
422 | 1 | View Code Duplication | if (function_exists($edit_func)) { |
423 | // execute the function |
||
424 | 1 | $options = explode('|', $this->getVar('options')); |
|
425 | 1 | $edit_form = $edit_func($options); |
|
426 | 1 | if (!$edit_form) { |
|
427 | 1 | return false; |
|
428 | } |
||
429 | } else { |
||
430 | 1 | return false; |
|
431 | } |
||
432 | 1 | return $edit_form; |
|
433 | } else { |
||
434 | 1 | return false; |
|
435 | } |
||
436 | } else { |
||
437 | return false; |
||
438 | } |
||
439 | } |
||
440 | |||
441 | /** |
||
442 | * Determine if this is a custom block |
||
443 | * |
||
444 | * @return bool true if this is a custom block |
||
445 | */ |
||
446 | 5 | public function isCustom() |
|
447 | { |
||
448 | 5 | return $this->getVar("block_type") === XoopsBlock::BLOCK_TYPE_CUSTOM; |
|
449 | } |
||
450 | |||
451 | /************ADDED**************/ |
||
452 | |||
453 | |||
454 | /** |
||
455 | * Build Block |
||
456 | * |
||
457 | * @return array|bool |
||
458 | */ |
||
459 | 2 | public function buildBlock() |
|
501 | } |
||
502 |
On one hand,
eval
might be exploited by malicious users if they somehow manage to inject dynamic content. On the other hand, with the emergence of faster PHP runtimes like the HHVM,eval
prevents some optimization that they perform.