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) { |
|
| 43 | 1 | $params = array_merge($filterTags, $filterTags); |
|
| 44 | 1 | array_unshift($params, $userId); |
|
| 45 | 1 | $not_in = ''; |
|
| 46 | 1 | if (!empty($filterTags)) { |
|
| 47 | $exist_clause = " AND exists (select 1 from `*PREFIX*bookmarks_tags` |
||
| 48 | `t2` where `t2`.`bookmark_id` = `t`.`bookmark_id` and `tag` = ?) "; |
||
| 49 | |||
| 50 | $not_in = ' AND `tag` not in (' . implode(',', array_fill(0, count($filterTags), '?')) . ')' . |
||
| 51 | str_repeat($exist_clause, count($filterTags)); |
||
| 52 | } |
||
| 53 | $sql = 'SELECT tag, count(*) as nbr from *PREFIX*bookmarks_tags t ' . |
||
| 54 | ' WHERE EXISTS( SELECT 1 from *PREFIX*bookmarks bm where t.bookmark_id = bm.id and user_id = ?) ' . |
||
| 55 | 1 | $not_in . |
|
| 56 | 1 | ' GROUP BY `tag` ORDER BY `nbr` DESC '; |
|
| 57 | |||
| 58 | 1 | $query = $db->prepareQuery($sql, $limit, $offset); |
|
| 59 | 1 | $tags = $query->execute($params)->fetchAll(); |
|
| 60 | 1 | return $tags; |
|
| 61 | } |
||
| 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) { |
|
| 71 | 2 | $CONFIG_DBTYPE = \OCP\Config::getSystemValue('dbtype', 'sqlite'); |
|
| 72 | 2 | if ($CONFIG_DBTYPE == 'pgsql') { |
|
| 73 | $group_fct = 'array_agg(`tag`)'; |
||
| 74 | } else { |
||
| 75 | 2 | $group_fct = 'GROUP_CONCAT(`tag`)'; |
|
| 76 | } |
||
| 77 | 2 | $sql = "SELECT *, (select $group_fct from `*PREFIX*bookmarks_tags` where `bookmark_id` = `b`.`id`) as `tags` |
|
| 78 | FROM `*PREFIX*bookmarks` `b` |
||
| 79 | 2 | WHERE `user_id` = ? AND `id` = ?"; |
|
| 80 | 2 | $query = $db->prepareQuery($sql); |
|
| 81 | 2 | $result = $query->execute(array($userId, $id))->fetchRow(); |
|
| 82 | 2 | $result['tags'] = explode(',', $result['tags']); |
|
| 83 | 2 | return $result; |
|
| 84 | } |
||
| 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( |
|
| 120 | $userid, IDb $db, $offset, $sqlSortColumn, $filters, $filterTagOnly, $limit = 10, $public = false, $requestedAttributes = null, $tagFilterConjunction = "and") { |
||
| 121 | |||
| 122 | 3 | $CONFIG_DBTYPE = \OCP\Config::getSystemValue('dbtype', 'sqlite'); |
|
| 123 | 3 | if (is_string($filters)) { |
|
| 124 | $filters = array($filters); |
||
| 125 | } |
||
| 126 | |||
| 127 | 3 | $toSelect = '*'; |
|
| 128 | 3 | $tableAttributes = array('id', 'url', 'title', 'user_id', 'description', |
|
| 129 | 'public', 'added', 'lastmodified', 'clickcount',); |
||
| 130 | |||
| 131 | 3 | $returnTags = true; |
|
| 132 | |||
| 133 | 3 | if ($requestedAttributes != null) { |
|
| 134 | |||
| 135 | 1 | $key = array_search('tags', $requestedAttributes); |
|
| 136 | 1 | if ($key == false) { |
|
| 137 | 1 | $returnTags = false; |
|
| 138 | } else { |
||
| 139 | unset($requestedAttributes[$key]); |
||
| 140 | } |
||
| 141 | |||
| 142 | 1 | $toSelect = implode(",", array_intersect($tableAttributes, $requestedAttributes)); |
|
| 143 | } |
||
| 144 | |||
| 145 | 3 | if ($CONFIG_DBTYPE == 'pgsql') { |
|
| 146 | $sql = "SELECT " . $toSelect . " FROM (SELECT *, (select array_to_string(array_agg(`tag`),',') |
||
| 147 | from `*PREFIX*bookmarks_tags` where `bookmark_id` = `b2`.`id`) as `tags` |
||
| 148 | FROM `*PREFIX*bookmarks` `b2` |
||
| 149 | WHERE `user_id` = ? ) as `b` WHERE true "; |
||
| 150 | } else { |
||
| 151 | 3 | $sql = "SELECT " . $toSelect . ", (SELECT GROUP_CONCAT(`tag`) from `*PREFIX*bookmarks_tags` |
|
| 152 | WHERE `bookmark_id` = `b`.`id`) as `tags` |
||
| 153 | FROM `*PREFIX*bookmarks` `b` |
||
| 154 | 3 | WHERE `user_id` = ? "; |
|
| 155 | } |
||
| 156 | |||
| 157 | 3 | $params = array($userid); |
|
| 158 | |||
| 159 | 3 | if ($public) { |
|
| 160 | 1 | $sql .= ' AND public = 1 '; |
|
| 161 | } |
||
| 162 | |||
| 163 | 3 | if (count($filters) > 0) { |
|
| 164 | 2 | Bookmarks::findBookmarksBuildFilter($sql, $params, $filters, $filterTagOnly, $tagFilterConjunction, $CONFIG_DBTYPE); |
|
| 165 | } |
||
| 166 | |||
| 167 | 3 | if (!in_array($sqlSortColumn, $tableAttributes)) { |
|
| 168 | 1 | $sqlSortColumn = 'lastmodified'; |
|
| 169 | } |
||
| 170 | 3 | $sql .= " ORDER BY " . $sqlSortColumn . " DESC "; |
|
| 171 | 3 | if ($limit == -1 || $limit === false) { |
|
| 172 | 3 | $limit = null; |
|
| 173 | 3 | $offset = null; |
|
| 174 | } |
||
| 175 | |||
| 176 | 3 | $query = $db->prepareQuery($sql, $limit, $offset); |
|
| 177 | 3 | $results = $query->execute($params)->fetchAll(); |
|
| 178 | 3 | $bookmarks = array(); |
|
| 179 | 3 | foreach ($results as $result) { |
|
| 180 | 3 | if ($returnTags) { |
|
| 181 | 2 | $result['tags'] = explode(',', $result['tags']); |
|
| 182 | } else { |
||
| 183 | 1 | unset($result['tags']); |
|
| 184 | } |
||
| 185 | 3 | $bookmarks[] = $result; |
|
| 186 | } |
||
| 187 | 3 | return $bookmarks; |
|
| 188 | } |
||
| 189 | |||
| 190 | 2 | private static function findBookmarksBuildFilter(&$sql, &$params, $filters, $filterTagOnly, $tagFilterConjunction, $CONFIG_DBTYPE) { |
|
| 191 | 2 | $tagOrSearch = false; |
|
| 192 | 2 | $connectWord = 'AND'; |
|
| 193 | |||
| 194 | 2 | if ($tagFilterConjunction == 'or') { |
|
| 195 | 1 | $tagOrSearch = true; |
|
| 196 | 1 | $connectWord = 'OR'; |
|
| 197 | } |
||
| 198 | |||
| 199 | 2 | if ($filterTagOnly) { |
|
| 200 | 2 | if ($tagOrSearch) { |
|
| 201 | 1 | $sql .= 'AND ('; |
|
| 202 | } else { |
||
| 203 | 1 | $sql .= 'AND'; |
|
| 204 | } |
||
| 205 | $exist_clause = " exists (SELECT `id` FROM `*PREFIX*bookmarks_tags` |
||
| 206 | 2 | `t2` WHERE `t2`.`bookmark_id` = `b`.`id` AND `tag` = ?) "; |
|
| 207 | 2 | $sql .= str_repeat($exist_clause . $connectWord, count($filters)); |
|
| 208 | 2 | if ($tagOrSearch) { |
|
| 209 | 1 | $sql = rtrim($sql, 'OR'); |
|
| 210 | 1 | $sql .= ')'; |
|
| 211 | } else { |
||
| 212 | 1 | $sql = rtrim($sql, 'AND'); |
|
| 213 | } |
||
| 214 | 2 | $params = array_merge($params, $filters); |
|
| 215 | } else { |
||
| 216 | if ($CONFIG_DBTYPE == 'mysql') { //Dirty hack to allow usage of alias in where |
||
| 217 | $sql .= ' HAVING true '; |
||
| 218 | } |
||
| 219 | foreach ($filters as $filter) { |
||
| 220 | if ($CONFIG_DBTYPE == 'mysql') { |
||
| 221 | $sql .= ' AND lower( concat(url,title,description,IFNULL(tags,\'\') )) like ? '; |
||
| 222 | } else { |
||
| 223 | $sql .= ' AND lower(url || title || description || ifnull(tags,\'\') ) like ? '; |
||
| 224 | } |
||
| 225 | $params[] = '%' . strtolower($filter) . '%'; |
||
| 226 | } |
||
| 227 | } |
||
| 228 | 2 | } |
|
| 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) { |
||
| 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
$irelandis not defined by the methodfinale(...).The most likely cause is that the parameter was changed, but the annotation was not.