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 ListTable 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 ListTable, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
21 | class ListTable |
||
22 | { |
||
23 | use UtilContainerAwareTrait; |
||
24 | |||
25 | |||
26 | /** |
||
27 | * If column in data and title does not match, fitMode option will |
||
28 | * determine which columns will be used, its value defines here. |
||
29 | */ |
||
30 | // Fit to title, drop data whose index is not in title index |
||
31 | const FIT_TO_TITLE = 0; |
||
32 | |||
33 | // Fit to data, drop title whose index is not in data index |
||
34 | const FIT_TO_DATA = 1; |
||
35 | |||
36 | // Fit to intersection of title and data, got fewest column |
||
37 | const FIT_INTERSECTION = 2; |
||
38 | |||
39 | // Fit to union of title and data, got most column |
||
40 | const FIT_UNION = 3; |
||
41 | |||
42 | /** |
||
43 | * Config array |
||
44 | * |
||
45 | * @var array |
||
46 | */ |
||
47 | protected $configs = []; |
||
48 | |||
49 | /** |
||
50 | * Information generated in treatment |
||
51 | * |
||
52 | * Default value is given according to default config. |
||
53 | * |
||
54 | * @var array |
||
55 | */ |
||
56 | protected $info = [ |
||
57 | // Class for root element |
||
58 | 'class' => 'ListTable', |
||
59 | // Class prefix for non-root elements |
||
60 | 'classPrefix' => 'ListTable-', |
||
61 | // Id of root element |
||
62 | 'id' => 'ListTable-1', |
||
63 | // Id prefix for non-root elements |
||
64 | 'idPrefix' => 'ListTable-1-', |
||
65 | // Orderby enabled column and default direction |
||
66 | 'orderByColumn' => [], |
||
67 | // Current page number |
||
68 | 'page' => 1, |
||
69 | // Max page number |
||
70 | 'pageMax' => 1, |
||
71 | // Parsed pager text |
||
72 | 'pagerTextBody' => '', |
||
73 | 'totalRows' => -1, |
||
74 | ]; |
||
75 | |||
76 | /** |
||
77 | * List data array |
||
78 | * |
||
79 | * @var array |
||
80 | */ |
||
81 | protected $listData = []; |
||
82 | |||
83 | /** |
||
84 | * List title array, show as table title |
||
85 | * |
||
86 | * @var array |
||
87 | */ |
||
88 | protected $listTitle = []; |
||
89 | |||
90 | /** |
||
91 | * Page url param array |
||
92 | * |
||
93 | * @var array |
||
94 | */ |
||
95 | protected $param = []; |
||
96 | |||
97 | /** |
||
98 | * Template object |
||
99 | * |
||
100 | * @var Smarty |
||
101 | */ |
||
102 | protected $tpl = null; |
||
103 | |||
104 | /** |
||
105 | * Prefix of var assigned to template |
||
106 | * |
||
107 | * Should keep sync with var name in template file, in most case this |
||
108 | * property need not change. |
||
109 | * |
||
110 | * @var string |
||
111 | */ |
||
112 | protected $tplVarPrefix = 'listTable'; |
||
113 | |||
114 | /** |
||
115 | * Array of url, computed for show in tpl |
||
116 | * |
||
117 | * { |
||
118 | * base : Original page url |
||
119 | * form : Page jump form target url |
||
120 | * obCur : Link on current orderBy head, to reverse order |
||
121 | * obOther : Link on in-active orderBy head(their default dir add in tpl) |
||
122 | * pageFirst : First page link |
||
123 | * pageLast : Last page link |
||
124 | * pageNext : Next page link |
||
125 | * pagePrev : Prev page link |
||
126 | * } |
||
127 | * |
||
128 | * @var array |
||
129 | */ |
||
130 | protected $url = [ |
||
131 | 'base' => '', |
||
132 | 'form' => '', |
||
133 | 'obCur' => '', |
||
134 | 'obOther' => '', |
||
135 | 'pageFirst' => '', |
||
136 | 'pageLast' => '', |
||
137 | 'pageNext' => '', |
||
138 | 'pagePrev' => '', |
||
139 | ]; |
||
140 | |||
141 | |||
142 | /** |
||
143 | * Constructor |
||
144 | * |
||
145 | * If there are multiple list to show in one page, MUST set 'id' in |
||
146 | * config. |
||
147 | * |
||
148 | * @param Smarty $tpl |
||
149 | * @param array $configs |
||
150 | */ |
||
151 | public function __construct($tpl, array $configs = []) |
||
166 | |||
167 | |||
168 | /** |
||
169 | * Fit each row in data with given keys |
||
170 | * |
||
171 | * If row index is not in given keys, it will be dropped. |
||
172 | * If given keys is not in row index, it will be created with value |
||
173 | * $this->configs['fitEmpty']. |
||
174 | * |
||
175 | * @param array $key |
||
176 | */ |
||
177 | protected function fitData(array $key) |
||
217 | |||
218 | |||
219 | /** |
||
220 | * Fit title with given keys |
||
221 | * |
||
222 | * Drop title value not in given keys, and create new if given keys is not |
||
223 | * exists in title array. |
||
224 | * |
||
225 | * @param array $key |
||
226 | */ |
||
227 | protected function fitTitle(array $key) |
||
244 | |||
245 | |||
246 | /** |
||
247 | * Fit data and title if their key are different |
||
248 | * |
||
249 | * Notice: data have multi row(2 dim), and 2nd dimension must use assoc |
||
250 | * index. Title have only 1 row(1 dim), integer or assoc indexed. |
||
251 | * |
||
252 | * @see $config['fitMode'] |
||
253 | */ |
||
254 | protected function fitTitleWithData() |
||
294 | |||
295 | |||
296 | /** |
||
297 | * Format list data use closure function |
||
298 | * |
||
299 | * Closure function $formatFunction must have 1 param and define as |
||
300 | * reference, eg: function (&row) {}. |
||
301 | * |
||
302 | * @param callback $formatFunction |
||
303 | * @return $this |
||
304 | */ |
||
305 | public function formatData($formatFunction) |
||
314 | |||
315 | |||
316 | /** |
||
317 | * Generate url with some modification |
||
318 | * |
||
319 | * @param array $modify |
||
320 | * @param array $exclude |
||
321 | * @return string |
||
322 | * @see $this->param |
||
323 | */ |
||
324 | protected function genUrl($modify = null, $exclude = null) |
||
347 | |||
348 | |||
349 | /** |
||
350 | * Get full output html |
||
351 | * |
||
352 | * @return string |
||
353 | */ |
||
354 | public function getHtml() |
||
361 | |||
362 | |||
363 | /** |
||
364 | * Get config for generate SQL |
||
365 | * |
||
366 | * The result will use as config in SqlGenerator, include: |
||
367 | * - LIMIT |
||
368 | * - ORDERBY |
||
369 | * |
||
370 | * When there are multiple list on single page, second list must set |
||
371 | * $forcenew to true. |
||
372 | * |
||
373 | * @param boolean $forcenew |
||
374 | * @return array |
||
375 | * @see Fwlib\Db\SqlGenerator |
||
376 | */ |
||
377 | public function getSqlConfig($forcenew = false) |
||
394 | |||
395 | |||
396 | /** |
||
397 | * Read http param and parse |
||
398 | * |
||
399 | * @param boolean $forcenew |
||
400 | * @return $this |
||
401 | */ |
||
402 | protected function readRequest($forcenew = false) |
||
450 | |||
451 | |||
452 | /** |
||
453 | * Set single config |
||
454 | * |
||
455 | * @param string $key |
||
456 | * @param mixed $value |
||
457 | * @return ListTable |
||
458 | */ |
||
459 | public function setConfig($key, $value) |
||
472 | |||
473 | |||
474 | /** |
||
475 | * Set multiple configs |
||
476 | * |
||
477 | * @param array $configs |
||
478 | * @return ListTable |
||
479 | */ |
||
480 | public function setConfigs(array $configs) |
||
496 | |||
497 | |||
498 | /** |
||
499 | * Set list data and title |
||
500 | * |
||
501 | * @param array $listData |
||
502 | * @param array $listTitle |
||
503 | * @param boolean $updateTotalRows |
||
504 | * @return $this |
||
505 | */ |
||
506 | public function setData($listData, $listTitle = null, $updateTotalRows = false) |
||
522 | |||
523 | |||
524 | /** |
||
525 | * Set db query |
||
526 | * |
||
527 | * Will query total rows and list data by set db connection and config, |
||
528 | * will overwrite exists $listData. |
||
529 | * |
||
530 | * @param Adodb $db |
||
531 | * @param array $config |
||
532 | * @return $this |
||
533 | */ |
||
534 | public function setDbQuery($db, $config) |
||
552 | |||
553 | |||
554 | /** |
||
555 | * Set default configs |
||
556 | */ |
||
557 | protected function setDefaultConfigs() |
||
690 | |||
691 | |||
692 | /** |
||
693 | * Set id and class of list table |
||
694 | * |
||
695 | * Id and class should not have space or other special chars not allowed |
||
696 | * by js and css in it. |
||
697 | * |
||
698 | * @param string $id |
||
699 | * @param string $class |
||
700 | * @return $this |
||
701 | */ |
||
702 | public function setId($id, $class = null) |
||
744 | |||
745 | |||
746 | /** |
||
747 | * Set orderBy info |
||
748 | * |
||
749 | * Did not validate orderBy key exists in data or title array. |
||
750 | * |
||
751 | * @param mixed $key |
||
752 | * @param string $dir ASC/DESC |
||
753 | */ |
||
754 | public function setOrderBy($key = null, $dir = null) |
||
804 | |||
805 | |||
806 | /** |
||
807 | * Check and set current page |
||
808 | * |
||
809 | * @param int $page Page num param come from outer |
||
810 | * @return $this |
||
811 | */ |
||
812 | protected function setPage($page = null) |
||
842 | |||
843 | |||
844 | /** |
||
845 | * Set pager info |
||
846 | * |
||
847 | * Will execute even pager above/below are both disabled. |
||
848 | * |
||
849 | * @return $this |
||
850 | * @see $config |
||
851 | */ |
||
852 | protected function setPager() |
||
922 | |||
923 | |||
924 | /** |
||
925 | * Set ListTable title |
||
926 | * |
||
927 | * Usually used together with setDbQuery(), which need not set listData |
||
928 | * from outside(use setData() method). |
||
929 | * |
||
930 | * In common this method should call before setDbQuery(). |
||
931 | * |
||
932 | * @param array $title |
||
933 | * @return $this |
||
934 | */ |
||
935 | public function setTitle($title) |
||
941 | |||
942 | |||
943 | /** |
||
944 | * Set totalRows |
||
945 | * |
||
946 | * @param int $totalRows |
||
947 | * @return $this |
||
948 | */ |
||
949 | public function setTotalRows($totalRows) |
||
955 | } |
||
956 |
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.