Complex classes like Bookmarks 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 Bookmarks, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
31 | class Bookmarks { |
||
32 | |||
33 | /** |
||
34 | * @brief Finds all tags for bookmarks |
||
35 | * @param string $userId UserId |
||
36 | * @param IDb $db Database Interface |
||
37 | * @param filterTags array of tag to look for if empty then every tag |
||
38 | * @param offset integer offset |
||
39 | * @param limit integer of item to return |
||
40 | * @return Found Tags |
||
41 | */ |
||
42 | 1 | public static function findTags($userId, IDb $db, $filterTags = array(), $offset = 0, $limit = -1) { |
|
62 | |||
63 | /** |
||
64 | * @brief Finds Bookmark with certain ID |
||
65 | * @param int $id BookmarkId |
||
66 | * @param string $userId UserId |
||
67 | * @param IDb $db Database Interface |
||
68 | * @return array Specific Bookmark |
||
69 | */ |
||
70 | 2 | public static function findUniqueBookmark($id, $userId, IDb $db) { |
|
85 | |||
86 | /** |
||
87 | * @brief Check if an URL is bookmarked |
||
88 | * @param $url Url of a possible bookmark |
||
89 | * @param $userId UserId |
||
90 | * @param IDb $db Database Interface |
||
91 | * @return boolean if the url is already bookmarked |
||
92 | */ |
||
93 | 1 | public static function bookmarkExists($url, $userId, IDb $db) { |
|
104 | |||
105 | /** |
||
106 | * @brief Finds all bookmarks, matching the filter |
||
107 | * @param string $userid UserId |
||
108 | * @param IDb $db Database Interface |
||
109 | * @param int $offset offset |
||
110 | * @param string $sqlSortColumn result with this column |
||
111 | * @param string|array $filters filters can be: empty -> no filter, a string -> filter this, a string array -> filter for all strings |
||
112 | * @param bool $filterTagOnly true, filter affects only tags, else filter affects url, title and tags |
||
113 | * @param int $limit limit of items to return (default 10) if -1 or false then all items are returned |
||
114 | * @param bool $public check if only public bookmarks should be returned |
||
115 | * @param array $requestedAttributes select all the attributes that should be returned. default is * + tags |
||
116 | * @param string $tagFilterConjunction select wether the filterTagOnly should filter with an AND or an OR conjunction |
||
117 | * @return Collection of specified bookmarks |
||
118 | */ |
||
119 | 3 | public static function findBookmarks( |
|
189 | |||
190 | 2 | private static function findBookmarksBuildFilter(&$sql, &$params, $filters, $filterTagOnly, $tagFilterConjunction, $CONFIG_DBTYPE) { |
|
229 | |||
230 | /** |
||
231 | * @brief Delete bookmark with specific id |
||
232 | * @param string $userId UserId |
||
233 | * @param IDb $db Database Interface |
||
234 | * @param int $id Bookmark ID to delete |
||
235 | * @return boolean Success of operation |
||
236 | */ |
||
237 | 1 | public static function deleteUrl($userId, IDb $db, $id) { |
|
267 | |||
268 | /** |
||
269 | * @brief Rename a tag |
||
270 | * @param $userId UserId |
||
271 | * @param IDb $db Database Interface |
||
272 | * @param string $old Old Tag Name |
||
273 | * @param string $new New Tag Name |
||
274 | * @return boolean Success of operation |
||
275 | */ |
||
276 | public static function renameTag($userId, IDb $db, $old, $new) { |
||
277 | $user_id = $userId; |
||
278 | $CONFIG_DBTYPE = \OCP\Config::getSystemValue('dbtype', 'sqlite'); |
||
279 | |||
280 | |||
281 | if ($CONFIG_DBTYPE == 'sqlite' or $CONFIG_DBTYPE == 'sqlite3') { |
||
282 | // Update tags to the new label unless it already exists a tag like this |
||
283 | $query = $db->prepareQuery(" |
||
284 | UPDATE OR REPLACE `*PREFIX*bookmarks_tags` |
||
285 | SET `tag` = ? |
||
286 | WHERE `tag` = ? |
||
287 | AND exists( select `b`.`id` from `*PREFIX*bookmarks` `b` |
||
288 | WHERE `b`.`user_id` = ? AND `bookmark_id` = `b`.`id`) |
||
289 | "); |
||
290 | |||
291 | $params = array( |
||
292 | $new, |
||
293 | $old, |
||
294 | $user_id, |
||
295 | ); |
||
296 | |||
297 | $query->execute($params); |
||
298 | } else { |
||
299 | |||
300 | // Remove potentialy duplicated tags |
||
301 | $query = $db->prepareQuery(" |
||
302 | DELETE FROM `*PREFIX*bookmarks_tags` as `tgs` WHERE `tgs`.`tag` = ? |
||
303 | AND exists( SELECT `id` FROM `*PREFIX*bookmarks` WHERE `user_id` = ? |
||
304 | AND `tgs`.`bookmark_id` = `id`) |
||
305 | AND exists( SELECT `t`.`tag` FROM `*PREFIX*bookmarks_tags` `t` where `t`.`tag` = ? |
||
306 | AND `tgs`.`bookmark_id` = `t`.`bookmark_id`)"); |
||
307 | |||
308 | $params = array( |
||
309 | $new, |
||
310 | $user_id, |
||
311 | $new |
||
312 | ); |
||
313 | |||
314 | $query->execute($params); |
||
315 | |||
316 | |||
317 | // Update tags to the new label unless it already exists a tag like this |
||
318 | $query = $db->prepareQuery(" |
||
319 | UPDATE `*PREFIX*bookmarks_tags` |
||
320 | SET `tag` = ? |
||
321 | WHERE `tag` = ? |
||
322 | AND exists( SELECT `b`.`id` FROM `*PREFIX*bookmarks` `b` |
||
323 | WHERE `b`.`user_id` = ? AND `bookmark_id` = `b`.`id`) |
||
324 | "); |
||
325 | |||
326 | $params = array( |
||
327 | $new, |
||
328 | $old, |
||
329 | $user_id |
||
330 | ); |
||
331 | |||
332 | $query->execute($params); |
||
333 | } |
||
334 | |||
335 | |||
336 | return true; |
||
337 | } |
||
338 | |||
339 | /** |
||
340 | * @brief Delete a tag |
||
341 | * @param $userid UserId |
||
342 | * @param IDb $db Database Interface |
||
343 | * @param string $old Tag Name to delete |
||
344 | * @return boolean Success of operation |
||
345 | */ |
||
346 | public static function deleteTag($userid, IDb $db, $old) { |
||
363 | |||
364 | /** |
||
365 | * Edit a bookmark |
||
366 | * @param string $userid UserId |
||
367 | * @param IDb $db Database Interface |
||
368 | * @param int $id The id of the bookmark to edit |
||
369 | * @param string $url The url to set |
||
370 | * @param string $title Name of the bookmark |
||
371 | * @param array $tags Simple array of tags to qualify the bookmark (different tags are taken from values) |
||
372 | * @param string $description A longer description about the bookmark |
||
373 | * @param boolean $is_public True if the bookmark is publishable to not registered users |
||
374 | * @return null |
||
375 | */ |
||
376 | 1 | public static function editBookmark($userid, IDb $db, $id, $url, $title, $tags = array(), $description = '', $is_public = false) { |
|
417 | |||
418 | /** |
||
419 | * Add a bookmark |
||
420 | * @param string $userid UserId |
||
421 | * @param IDb $db Database Interface |
||
422 | * @param string $url |
||
423 | * @param string $title Name of the bookmark |
||
424 | * @param array $tags Simple array of tags to qualify the bookmark (different tags are taken from values) |
||
425 | * @param string $description A longer description about the bookmark |
||
426 | * @param boolean $public True if the bookmark is publishable to not registered users |
||
|
|||
427 | * @return int The id of the bookmark created |
||
428 | */ |
||
429 | 8 | public static function addBookmark($userid, IDb $db, $url, $title, $tags = array(), $description = '', $is_public = false) { |
|
485 | |||
486 | /** |
||
487 | * @brief Add a set of tags for a bookmark |
||
488 | * @param IDb $db Database Interface |
||
489 | * @param int $bookmarkID The bookmark reference |
||
490 | * @param array $tags Set of tags to add to the bookmark |
||
491 | * @return null |
||
492 | * */ |
||
493 | 8 | private static function addTags(IDb $db, $bookmarkID, $tags) { |
|
513 | |||
514 | /** |
||
515 | * @brief Import Bookmarks from html formatted file |
||
516 | * @param string $user User imported Bookmarks should belong to |
||
517 | * @param IDb $db Database Interface |
||
518 | * @param string $file Content to import |
||
519 | * @return null |
||
520 | * */ |
||
521 | public static function importFile($user, IDb $db, $file) { |
||
554 | |||
555 | /** |
||
556 | * @brief Load Url and receive Metadata (Title) |
||
557 | * @param string $url Url to load and analyze |
||
558 | * @return array Metadata for url; |
||
559 | * @throws \Exception |
||
560 | */ |
||
561 | 1 | public static function getURLMetadata($url) { |
|
604 | |||
605 | /** |
||
606 | * @brief Seperate Url String at comma charachter |
||
607 | * @param $line String of Tags |
||
608 | * @return array Array of Tags |
||
609 | * */ |
||
610 | public static function analyzeTagRequest($line) { |
||
619 | |||
620 | } |
||
621 |
This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.
Consider the following example. The parameter
$ireland
is not defined by the methodfinale(...)
.The most likely cause is that the parameter was changed, but the annotation was not.