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 cachelist 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 cachelist, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class cachelist |
||
17 | { |
||
18 | public $nCachelistId = 0; |
||
19 | public $reCachelist; |
||
20 | |||
21 | View Code Duplication | public function __construct($nNewCacheListId = ID_NEW, $nUserId = 0) |
|
|
|||
22 | { |
||
23 | $this->reCachelist = new rowEditor('cache_lists'); |
||
24 | $this->reCachelist->addPKInt('id', null, false, RE_INSERT_AUTOINCREMENT); |
||
25 | $this->reCachelist->addString('uuid', '', false, RE_INSERT_AUTOUUID); |
||
26 | $this->reCachelist->addInt('node', 0, false); |
||
27 | $this->reCachelist->addInt('user_id', $nUserId, false); |
||
28 | $this->reCachelist->addDate('date_created', time(), true, RE_INSERT_IGNORE); |
||
29 | $this->reCachelist->addDate('last_modified', time(), true, RE_INSERT_IGNORE); |
||
30 | $this->reCachelist->addDate('last_added', null, true); |
||
31 | $this->reCachelist->addString('name', '', false); |
||
32 | $this->reCachelist->addInt('is_public', 0, false); |
||
33 | $this->reCachelist->addString('description', '', false); |
||
34 | $this->reCachelist->addInt('desc_htmledit', 1, false); |
||
35 | $this->reCachelist->addString('password', '', false); |
||
36 | |||
37 | $this->nCachelistId = $nNewCacheListId + 0; |
||
38 | |||
39 | if ($nNewCacheListId == ID_NEW) { |
||
40 | $this->reCachelist->addNew(null); |
||
41 | } else { |
||
42 | $this->reCachelist->load($this->nCachelistId); |
||
43 | } |
||
44 | } |
||
45 | |||
46 | public function exist() |
||
47 | { |
||
48 | return $this->reCachelist->exist(); |
||
49 | } |
||
50 | |||
51 | public function getId() |
||
52 | { |
||
53 | return $this->nCachelistId; |
||
54 | } |
||
55 | |||
56 | public function getUUID() |
||
57 | { |
||
58 | return $this->reCachelist->getValue('uuid'); |
||
59 | } |
||
60 | |||
61 | public function setNode($value) |
||
62 | { |
||
63 | return $this->reCachelist->setValue('node', $value); |
||
64 | } |
||
65 | |||
66 | public function getNode() |
||
67 | { |
||
68 | return $this->reCachelist->getValue('node'); |
||
69 | } |
||
70 | |||
71 | public function getUserId() |
||
72 | { |
||
73 | return $this->reCachelist->getValue('user_id'); |
||
74 | } |
||
75 | |||
76 | public function isMyList() |
||
77 | { |
||
78 | global $login; |
||
79 | |||
80 | return $this->getUserId() == $login->userid; |
||
81 | } |
||
82 | |||
83 | public function getName() |
||
84 | { |
||
85 | return $this->reCachelist->getValue('name'); |
||
86 | } |
||
87 | |||
88 | // 0 = private, 1 = private & friends (not impl.), 2 = public, 3 = public + listing display |
||
89 | public function getVisibility() |
||
90 | { |
||
91 | return $this->reCachelist->getValue('is_public'); |
||
92 | } |
||
93 | |||
94 | // !! This method returns an error state instead of a success flag; false means "no error". |
||
95 | public function setNameAndVisibility($name, $visibility) |
||
96 | { |
||
97 | $name = trim($name); |
||
98 | if ($name == '') { |
||
99 | return ERROR_BAD_LISTNAME; |
||
100 | } |
||
101 | if (sql_value( |
||
102 | "SELECT `id` |
||
103 | FROM `cache_lists` |
||
104 | WHERE `user_id`='&1' AND `id`<>'&2' AND `name`='&3'", |
||
105 | false, |
||
106 | $this->getUserId(), |
||
107 | $this->getId(), |
||
108 | $name |
||
109 | )) { |
||
110 | // $this->getId() is 0 when creating a new list -> condition has no effect |
||
111 | return ERROR_DUPLICATE_LISTNAME; |
||
112 | } elseif ($visibility >= 2 && strlen($name) < 10) { |
||
113 | return ERROR_BAD_LISTNAME; |
||
114 | } |
||
115 | |||
116 | $error = !$this->reCachelist->setValue('name', trim($name)); |
||
117 | if ($visibility == 0 || $visibility == 2 || $visibility == 3) { |
||
118 | $error |= !$this->reCachelist->setValue('is_public', $visibility); |
||
119 | } |
||
120 | |||
121 | return $error; |
||
122 | } |
||
123 | |||
124 | // return description in HTML format |
||
125 | public function getDescription() |
||
126 | { |
||
127 | return $this->reCachelist->getValue('description'); |
||
128 | } |
||
129 | |||
130 | public function getDescriptionForDisplay() |
||
131 | { |
||
132 | return use_current_protocol_in_html(getDescription()); |
||
133 | } |
||
134 | |||
135 | public function getDescHtmledit() |
||
136 | { |
||
137 | return $this->reCachelist->getValue('desc_htmledit'); |
||
138 | } |
||
139 | |||
140 | // set description in HTML format, must be purified! |
||
141 | public function setDescription($desc, $htmledit) |
||
142 | { |
||
143 | $this->reCachelist->setValue('desc_htmledit', $htmledit ? 1 : 0); |
||
144 | |||
145 | return $this->reCachelist->setValue('description', $desc); |
||
146 | } |
||
147 | |||
148 | public function setPassword($pw): void |
||
149 | { |
||
150 | $this->reCachelist->setValue('password', $pw); |
||
151 | } |
||
152 | |||
153 | public function getPassword() |
||
154 | { |
||
155 | return $this->reCachelist->getValue('password'); |
||
156 | } |
||
157 | |||
158 | public function getCachesCount() |
||
159 | { |
||
160 | return sql_value( |
||
161 | " |
||
162 | SELECT `entries` FROM `stat_cache_lists` |
||
163 | WHERE `stat_cache_lists`.`cache_list_id`='" . sql_escape($this->getId()) . "'", |
||
164 | 0 |
||
165 | ); |
||
166 | } |
||
167 | |||
168 | public function getWatchersCount() |
||
169 | { |
||
170 | return sql_value( |
||
171 | " |
||
172 | SELECT `watchers` FROM `stat_cache_lists` |
||
173 | WHERE `stat_cache_lists`.`cache_list_id`='" . sql_escape($this->getId()) . "'", |
||
174 | 0 |
||
175 | ); |
||
176 | } |
||
177 | |||
178 | public function save() |
||
179 | { |
||
180 | if ($this->getVisibility() > 0) { |
||
181 | $this->setPassword(''); |
||
182 | } |
||
183 | sql_slave_exclude(); |
||
184 | if ($this->reCachelist->save()) { |
||
185 | if ($this->getId() == ID_NEW) { |
||
186 | $this->nCachelistId = $this->reCachelist->getValue('id'); |
||
187 | } |
||
188 | |||
189 | return true; |
||
190 | } |
||
191 | |||
192 | return false; |
||
193 | } |
||
194 | |||
195 | // get and set list contents |
||
196 | |||
197 | // locked/hidden caches may be added to a list by the owner or an administrator, |
||
198 | // but getCaches() will return visible==0 if the list is queried by someone else. |
||
199 | // The 'visible' flag MUST be evaluated and the cache name must not be shown |
||
200 | // if it is 0. This also ensures that cache names are hidden if a cache is locked/hidden |
||
201 | // after being added to a list. |
||
202 | |||
203 | public function getCaches() |
||
204 | { |
||
205 | global $login; |
||
206 | $login->verify(); |
||
207 | |||
208 | $rs = sql( |
||
209 | " |
||
210 | SELECT `cache_list_items`.`cache_id`, `caches`.`wp_oc`, `caches`.`name`, |
||
211 | `caches`.`type`, `caches`.`status`, |
||
212 | (`cache_status`.`allow_user_view` OR `caches`.`user_id`='&2' OR '&3') AS `visible`, |
||
213 | `ca`.`attrib_id` IS NOT NULL AS `oconly` |
||
214 | FROM `cache_list_items` |
||
215 | LEFT JOIN `caches` ON `caches`.`cache_id`=`cache_list_items`.`cache_id` |
||
216 | LEFT JOIN `cache_status` ON `cache_status`.`id`=`caches`.`status` |
||
217 | LEFT JOIN `caches_attributes` `ca` ON `ca`.`cache_id`=`caches`.`cache_id` AND `ca`.`attrib_id`=6 |
||
218 | WHERE `cache_list_items`.`cache_list_id` = '&1' |
||
219 | ORDER BY `caches`.`name`", |
||
220 | $this->nCachelistId, |
||
221 | $login->userid, |
||
222 | ($login->admin & ADMIN_USER) ? 1 : 0 |
||
223 | ); |
||
224 | |||
225 | return sql_fetch_assoc_table($rs); |
||
226 | } |
||
227 | |||
228 | public function addCacheByWP($wp) |
||
229 | { |
||
230 | $cache = cache::fromWP($wp); |
||
231 | if (!is_object($cache)) { |
||
232 | return false; |
||
233 | } |
||
234 | |||
235 | return $this->addCache($cache); |
||
236 | } |
||
237 | |||
238 | // returns true if all wayPoints were valid, or an array of invalid wayPoints |
||
239 | public function addCachesByWPs($wps) |
||
240 | { |
||
241 | $wpa = explode(' ', trim($wps)); |
||
242 | $non_added_wps = []; |
||
243 | foreach ($wpa as $wp) { |
||
244 | $wp = trim($wp); |
||
245 | if ($wp) { |
||
246 | $result = $this->addCacheByWP($wp); |
||
247 | if ($result !== true) { |
||
248 | $non_added_wps[] = $wp; |
||
249 | } |
||
250 | } |
||
251 | } |
||
252 | if (count($non_added_wps)) { |
||
253 | return $non_added_wps; |
||
254 | } |
||
255 | |||
256 | return true; |
||
257 | } |
||
258 | |||
259 | public function addCachesByIDs($cache_ids) |
||
270 | |||
271 | public function addCacheByID($cache_id) |
||
272 | { |
||
273 | return $this->addCache(new cache($cache_id)); |
||
274 | } |
||
275 | |||
276 | public function addCache($cache) |
||
277 | { |
||
278 | if (!$cache->exist() || !$cache->allowView()) { |
||
279 | return false; |
||
280 | } |
||
281 | sql( |
||
282 | " |
||
283 | INSERT IGNORE INTO `cache_list_items` (`cache_list_id`, `cache_id`) |
||
284 | VALUES ('&1', '&2')", |
||
285 | $this->nCachelistId, |
||
286 | $cache->getCacheId() |
||
287 | ); |
||
288 | |||
289 | return true; |
||
290 | } |
||
291 | |||
292 | public function removeCacheById($cache_id): void |
||
293 | { |
||
294 | sql( |
||
295 | "DELETE FROM `cache_list_items` WHERE `cache_list_id`='&1' AND `cache_id`='&2'", |
||
300 | |||
301 | // watching, bookmarking and access tests |
||
302 | public function watch($watch): void |
||
329 | |||
330 | public function isWatchedByMe() |
||
342 | |||
343 | public function bookmark($pw): void |
||
361 | |||
362 | public function unbookmark(): void |
||
373 | |||
374 | public function allowView($pw = '') |
||
397 | |||
398 | // get list of lists -- public static functions |
||
399 | public static function getMyLists() |
||
405 | |||
406 | public static function getListsWatchedByMe() |
||
416 | |||
417 | public static function getBookmarkedLists() |
||
427 | |||
428 | public static function getPublicListCount($namelike = '', $userlike = '') |
||
442 | |||
443 | public static function getPublicLists($startat = 0, $maxitems = PHP_INT_MAX, $namelike = '', $userlike = '') |
||
455 | |||
456 | public static function getPublicListsOf($userid) |
||
462 | |||
463 | // If $all is false, only own lists and public lists of the cache owner will be returned. |
||
464 | public static function getListsByCacheId($cacheid, $all) |
||
498 | |||
499 | public static function getListById($listid) |
||
510 | |||
511 | private static function getLists( |
||
552 | |||
553 | // other |
||
554 | public static function getMyLastAddedToListId() |
||
573 | |||
574 | public static function watchingCacheByListsCount($userId, $cacheId) |
||
589 | } |
||
590 |
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.