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 Upgrade_220 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 Upgrade_220, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
20 | class Upgrade_220 extends XoopsUpgrade |
||
21 | { |
||
22 | public $tasks = array('config', 'profile', 'block'/*, 'pm', 'module'*/); |
||
23 | |||
24 | public function __construct() |
||
25 | { |
||
26 | parent::__construct(basename(__DIR__)); |
||
27 | } |
||
28 | |||
29 | /** |
||
30 | * Check if config category already removed |
||
31 | * |
||
32 | */ |
||
33 | public function check_config() |
||
34 | { |
||
35 | $sql = 'SHOW COLUMNS FROM `' . $GLOBALS['xoopsDB']->prefix('configcategory') . "` LIKE 'confcat_modid'"; |
||
36 | $result = $GLOBALS['xoopsDB']->queryF($sql); |
||
37 | if (!$result) { |
||
38 | return true; |
||
39 | } |
||
40 | return !($GLOBALS['xoopsDB']->getRowsNum($result) > 0); |
||
41 | } |
||
42 | |||
43 | /** |
||
44 | * Check if user profile table already converted |
||
45 | * |
||
46 | */ |
||
47 | public function check_profile() |
||
48 | { |
||
49 | $module_handler = xoops_getHandler('module'); |
||
50 | if (!$profile_module = $module_handler->getByDirname('profile')) { |
||
51 | return true; |
||
52 | } |
||
53 | $sql = 'SHOW COLUMNS FROM ' . $GLOBALS['xoopsDB']->prefix('users') . " LIKE 'posts'"; |
||
54 | $result = $GLOBALS['xoopsDB']->queryF($sql); |
||
55 | if (!$result) { |
||
56 | return false; |
||
57 | } |
||
58 | |||
59 | return !($GLOBALS['xoopsDB']->getRowsNum($result) == 0); |
||
60 | } |
||
61 | |||
62 | /** |
||
63 | * Check if block table already converted |
||
64 | * |
||
65 | */ |
||
66 | public function check_block() |
||
67 | { |
||
68 | $sql = "SHOW TABLES LIKE '" . $GLOBALS['xoopsDB']->prefix('block_instance') . "'"; |
||
69 | $result = $GLOBALS['xoopsDB']->queryF($sql); |
||
70 | if (!$result) { |
||
71 | return true; |
||
72 | } |
||
73 | |||
74 | return !($GLOBALS['xoopsDB']->getRowsNum($result) > 0); |
||
75 | } |
||
76 | |||
77 | /** |
||
78 | * @return bool |
||
79 | */ |
||
80 | public function apply() |
||
81 | { |
||
82 | if (empty($_GET['upd220'])) { |
||
83 | $this->logs[] = _CONFIRM_UPGRADE_220; |
||
84 | $res = false; |
||
85 | } else { |
||
86 | $res = parent::apply(); |
||
87 | } |
||
88 | |||
89 | return $res; |
||
90 | } |
||
91 | |||
92 | /** |
||
93 | * @return bool |
||
94 | */ |
||
95 | public function apply_config() |
||
96 | { |
||
97 | global $xoopsDB; |
||
98 | |||
99 | $result = true; |
||
100 | |||
101 | //Set core configuration back to zero for system module |
||
102 | $xoopsDB->queryF('UPDATE `' . $xoopsDB->prefix('config') . '` SET conf_modid = 0 WHERE conf_modid = 1'); |
||
103 | |||
104 | //Change debug modes so there can only be one active at any one time |
||
105 | $xoopsDB->queryF('UPDATE `' . $xoopsDB->prefix('config') . "` SET conf_formtype = 'select', conf_valuetype = 'int' WHERE conf_name = 'debug_mode'"); |
||
106 | |||
107 | //Reset category ID for non-system configs |
||
108 | $xoopsDB->queryF('UPDATE `' . $xoopsDB->prefix('config') . '` SET conf_catid = 0 WHERE conf_modid > 1 AND conf_catid > 0'); |
||
109 | |||
110 | // remove admin theme configuration item |
||
111 | $xoopsDB->queryF('DELETE FROM `' . $xoopsDB->prefix('config') . "` WHERE conf_name='theme_set_admin'"); |
||
112 | |||
113 | //Drop non-System config categories |
||
114 | $xoopsDB->queryF('DELETE FROM `' . $xoopsDB->prefix('configcategory') . '` WHERE confcat_modid > 1'); |
||
115 | |||
116 | //Drop category information fields added in 2.2 |
||
117 | $xoopsDB->queryF('ALTER TABLE `' . $xoopsDB->prefix('configcategory') . '` DROP `confcat_nameid`, DROP `confcat_description`, DROP `confcat_modid`'); |
||
118 | |||
119 | // Re-add user configuration category |
||
120 | $xoopsDB->queryF('INSERT INTO `' . $xoopsDB->prefix('configcategory') . "` (confcat_id, confcat_name, confcat_order) VALUES (2, '_MD_AM_USERSETTINGS', 2)"); |
||
121 | |||
122 | //Rebuild user configuration items |
||
123 | //Get values from Profile module |
||
124 | $profile_config_arr = array(); |
||
125 | $profile_config_arr['minpass'] = 5; |
||
126 | $profile_config_arr['minuname'] = 3; |
||
127 | $profile_config_arr['new_user_notify'] = 1; |
||
128 | $profile_config_arr['new_user_notify_group'] = XOOPS_GROUP_ADMIN; |
||
129 | $profile_config_arr['activation_type'] = 0; |
||
130 | $profile_config_arr['activation_group'] = XOOPS_GROUP_ADMIN; |
||
131 | $profile_config_arr['uname_test_level'] = 0; |
||
132 | $profile_config_arr['avatar_allow_upload'] = 0; |
||
133 | $profile_config_arr['avatar_width'] = 80; |
||
134 | $profile_config_arr['avatar_height'] = 80; |
||
135 | $profile_config_arr['avatar_maxsize'] = 35000; |
||
136 | $profile_config_arr['self_delete'] = 0; |
||
137 | $profile_config_arr['bad_unames'] = serialize(array('webmaster', '^xoops', '^admin')); |
||
138 | $profile_config_arr['bad_emails'] = serialize(array('xoops.org$')); |
||
139 | $profile_config_arr['maxuname'] = 10; |
||
140 | $profile_config_arr['avatar_minposts'] = 0; |
||
141 | $profile_config_arr['allow_chgmail'] = 0; |
||
142 | $profile_config_arr['reg_dispdsclmr'] = 0; |
||
143 | $profile_config_arr['reg_disclaimer'] = ''; |
||
144 | $profile_config_arr['allow_register'] = 1; |
||
145 | |||
146 | $module_handler = xoops_getHandler('module'); |
||
147 | $config_handler = xoops_getHandler('config'); |
||
148 | $profile_module = $module_handler->getByDirname('profile'); |
||
149 | if (is_object($profile_module)) { |
||
150 | $profile_config = $config_handler->getConfigs(new Criteria('conf_modid', $profile_module->getVar('mid'))); |
||
151 | foreach (array_keys($profile_config) as $i) { |
||
152 | $profile_config_arr[$profile_config[$i]->getVar('conf_name')] = $profile_config[$i]->getVar('conf_value', 'n'); |
||
153 | } |
||
154 | } |
||
155 | |||
156 | $xoopsDB->queryF('INSERT INTO `' . $xoopsDB->prefix('config') . '` (conf_modid, conf_catid, conf_name, conf_title, conf_value, conf_desc, conf_formtype, conf_valuetype, conf_order) VALUES ' . " (0, 2, 'minpass', '_MD_AM_MINPASS', " . $xoopsDB->quote($profile_config_arr['minpass']) . ", '_MD_AM_MINPASSDSC', 'textbox', 'int', 1)," . " (0, 2, 'minuname', '_MD_AM_MINUNAME', " . $xoopsDB->quote($profile_config_arr['minuname']) . ", '_MD_AM_MINUNAMEDSC', 'textbox', 'int', 2)," . " (0, 2, 'new_user_notify', '_MD_AM_NEWUNOTIFY', " . $xoopsDB->quote($profile_config_arr['new_user_notify']) . ", '_MD_AM_NEWUNOTIFYDSC', 'yesno', 'int', 4)," . " (0, 2, 'new_user_notify_group', '_MD_AM_NOTIFYTO', " . $xoopsDB->quote($profile_config_arr['new_user_notify_group']) . ", '_MD_AM_NOTIFYTODSC', 'group', 'int', 6)," . " (0, 2, 'activation_type', '_MD_AM_ACTVTYPE', " . $xoopsDB->quote($profile_config_arr['activation_type']) . ", '_MD_AM_ACTVTYPEDSC', 'select', 'int', 8)," . " (0, 2, 'activation_group', '_MD_AM_ACTVGROUP', " . $xoopsDB->quote($profile_config_arr['activation_group']) . ", '_MD_AM_ACTVGROUPDSC', 'group', 'int', 10)," . " (0, 2, 'uname_test_level', '_MD_AM_UNAMELVL', " . $xoopsDB->quote($profile_config_arr['uname_test_level']) . ", '_MD_AM_UNAMELVLDSC', 'select', 'int', 12)," . " (0, 2, 'avatar_allow_upload', '_MD_AM_AVATARALLOW', " . $xoopsDB->quote($profile_config_arr['avatar_allow_upload']) . ", '_MD_AM_AVATARALWDSC', 'yesno', 'int', 14)," . " (0, 2, 'avatar_width', '_MD_AM_AVATARW', " . $xoopsDB->quote($profile_config_arr['avatar_width']) . ", '_MD_AM_AVATARWDSC', 'textbox', 'int', 16)," . " (0, 2, 'avatar_height', '_MD_AM_AVATARH', " . $xoopsDB->quote($profile_config_arr['avatar_height']) . ", '_MD_AM_AVATARHDSC', 'textbox', 'int', 18)," . " (0, 2, 'avatar_maxsize', '_MD_AM_AVATARMAX', " . $xoopsDB->quote($profile_config_arr['avatar_maxsize']) . ", '_MD_AM_AVATARMAXDSC', 'textbox', 'int', 20)," . " (0, 2, 'self_delete', '_MD_AM_SELFDELETE', " . $xoopsDB->quote($profile_config_arr['self_delete']) . ", '_MD_AM_SELFDELETEDSC', 'yesno', 'int', 22)," . " (0, 2, 'bad_unames', '_MD_AM_BADUNAMES', " . $xoopsDB->quote($profile_config_arr['bad_unames']) . ", '_MD_AM_BADUNAMESDSC', 'textarea', 'array', 24)," . " (0, 2, 'bad_emails', '_MD_AM_BADEMAILS', " . $xoopsDB->quote($profile_config_arr['bad_emails']) . ", '_MD_AM_BADEMAILSDSC', 'textarea', 'array', 26)," . " (0, 2, 'maxuname', '_MD_AM_MAXUNAME', " . $xoopsDB->quote($profile_config_arr['maxuname']) . ", '_MD_AM_MAXUNAMEDSC', 'textbox', 'int', 3)," . " (0, 2, 'avatar_minposts', '_MD_AM_AVATARMP', " . $xoopsDB->quote($profile_config_arr['avatar_minposts']) . ", '_MD_AM_AVATARMPDSC', 'textbox', 'int', 15)," . " (0, 2, 'allow_chgmail', '_MD_AM_ALLWCHGMAIL', " . $xoopsDB->quote($profile_config_arr['allow_chgmail']) . ", '_MD_AM_ALLWCHGMAILDSC', 'yesno', 'int', 3)," . " (0, 2, 'reg_dispdsclmr', '_MD_AM_DSPDSCLMR', " . $xoopsDB->quote($profile_config_arr['reg_dispdsclmr']) . ", '_MD_AM_DSPDSCLMRDSC', 'yesno', 'int', 30)," . " (0, 2, 'reg_disclaimer', '_MD_AM_REGDSCLMR', " . $xoopsDB->quote($profile_config_arr['reg_disclaimer']) . ", '_MD_AM_REGDSCLMRDSC', 'textarea', 'text', 32)," . " (0, 2, 'allow_register', '_MD_AM_ALLOWREG', " . $xoopsDB->quote($profile_config_arr['allow_register']) . ", '_MD_AM_ALLOWREGDSC', 'yesno', 'int', 0)"); |
||
157 | |||
158 | //Rebuild user configuration options |
||
159 | $criteria = new CriteriaCompo(new Criteria('conf_name', "('activation_type', 'uname_test_level')", 'IN')); |
||
160 | $criteria->add(new Criteria('conf_modid', 0)); |
||
161 | $criteria->setSort('conf_name'); |
||
162 | $criteria->setOrder('ASC'); |
||
163 | $configs = $config_handler->getConfigs($criteria); |
||
164 | $id_activation_type = $configs[0]->getVar('conf_id'); |
||
165 | $id_uname_test_level = $configs[1]->getVar('conf_id'); |
||
166 | $xoopsDB->queryF('INSERT INTO `' . $xoopsDB->prefix('configoption') . '` (confop_name, confop_value, conf_id) VALUES ' . " ('_MD_AM_USERACTV', '0', {$id_activation_type})," . " ('_MD_AM_AUTOACTV', '1', {$id_activation_type})," . " ('_MD_AM_ADMINACTV', '2', {$id_activation_type})," . " ('_MD_AM_STRICT', '0', {$id_uname_test_level})," . " ('_MD_AM_MEDIUM', '1', {$id_uname_test_level})," . " ('_MD_AM_LIGHT', '2', {$id_uname_test_level})"); |
||
167 | |||
168 | return $result; |
||
169 | } |
||
170 | |||
171 | /** |
||
172 | * @return bool |
||
173 | */ |
||
174 | public function apply_profile() |
||
175 | { |
||
176 | global $xoopsDB; |
||
177 | // Restore users table |
||
178 | $xoopsDB->queryF('ALTER TABLE `' . $xoopsDB->prefix('users') . "` |
||
179 | ADD url varchar(100) NOT NULL default '', |
||
180 | ADD user_regdate int(10) unsigned NOT NULL default '0', |
||
181 | ADD user_icq varchar(15) NOT NULL default '', |
||
182 | ADD user_from varchar(100) NOT NULL default '', |
||
183 | ADD user_sig tinytext, |
||
184 | ADD user_viewemail tinyint(1) unsigned NOT NULL default '0', |
||
185 | ADD actkey varchar(8) NOT NULL default '', |
||
186 | ADD user_aim varchar(18) NOT NULL default '', |
||
187 | ADD user_yim varchar(25) NOT NULL default '', |
||
188 | ADD user_msnm varchar(100) NOT NULL default '', |
||
189 | ADD posts mediumint(8) unsigned NOT NULL default '0', |
||
190 | ADD attachsig tinyint(1) unsigned NOT NULL default '0', |
||
191 | ADD theme varchar(100) NOT NULL default '', |
||
192 | ADD timezone_offset float(3,1) NOT NULL default '0.0', |
||
193 | ADD last_login int(10) unsigned NOT NULL default '0', |
||
194 | ADD umode varchar(10) NOT NULL default '', |
||
195 | ADD uorder tinyint(1) unsigned NOT NULL default '0', |
||
196 | ADD notify_method tinyint(1) NOT NULL default '1', |
||
197 | ADD notify_mode tinyint(1) NOT NULL default '0', |
||
198 | ADD user_occ varchar(100) NOT NULL default '', |
||
199 | ADD bio tinytext, |
||
200 | ADD user_intrest varchar(150) NOT NULL default '', |
||
201 | ADD user_mailok tinyint(1) unsigned NOT NULL default '1' |
||
202 | "); |
||
203 | |||
204 | // Copy data from profile table |
||
205 | $profile_fields = array( |
||
206 | 'url', |
||
207 | 'user_regdate', |
||
208 | 'user_icq', |
||
209 | 'user_from', |
||
210 | 'user_sig', |
||
211 | 'user_viewemail', |
||
212 | 'actkey', |
||
213 | 'user_aim', |
||
214 | 'user_yim', |
||
215 | 'user_msnm', |
||
216 | 'posts', |
||
217 | 'attachsig', |
||
218 | 'theme', |
||
219 | 'timezone_offset', |
||
220 | 'last_login', |
||
221 | 'umode', |
||
222 | 'uorder', |
||
223 | 'notify_method', |
||
224 | 'notify_mode', |
||
225 | 'user_occ', |
||
226 | 'bio', |
||
227 | 'user_intrest', |
||
228 | 'user_mailok'); |
||
229 | foreach ($profile_fields as $field) { |
||
230 | $xoopsDB->queryF('UPDATE `' . $xoopsDB->prefix('users') . '` u, `' . $xoopsDB->prefix('user_profile') . "` p SET u.{$field} = p.{$field} WHERE u.uid=p.profileid"); |
||
231 | } |
||
232 | |||
233 | //Set display name as real name |
||
234 | $xoopsDB->queryF('UPDATE `' . $xoopsDB->prefix('users') . "` SET name=uname WHERE name=''"); |
||
235 | //Set loginname as uname |
||
236 | $xoopsDB->queryF('UPDATE `' . $xoopsDB->prefix('users') . '` SET uname=loginname'); |
||
237 | //Drop loginname |
||
238 | $xoopsDB->queryF('ALTER TABLE `' . $xoopsDB->prefix('users') . '` DROP loginname'); |
||
239 | |||
240 | return true; |
||
241 | } |
||
242 | |||
243 | /** |
||
244 | * @param $block |
||
245 | * @param $blocks |
||
246 | * |
||
247 | * @return int|null|string |
||
248 | */ |
||
249 | public function _block_lookup($block, $blocks) |
||
250 | { |
||
251 | if ($block['show_func'] === 'b_system_custom_show') { |
||
252 | return 0; |
||
253 | } |
||
254 | |||
255 | foreach ($blocks as $key => $bk) { |
||
256 | if ($block['show_func'] == $bk['show_func'] && $block['edit_func'] == $bk['edit_func'] && $block['template'] == $bk['template']) { |
||
257 | return $key; |
||
258 | } |
||
259 | } |
||
260 | |||
261 | return null; |
||
262 | } |
||
263 | |||
264 | /** |
||
265 | * @return bool |
||
266 | */ |
||
267 | public function apply_block() |
||
431 | } |
||
432 | |||
433 | $upg = new Upgrade_220(); |
||
434 | return $upg; |
||
435 |
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.