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 User_Proxy 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 User_Proxy, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 35 | class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP { |
||
| 36 | private $backends = array(); |
||
| 37 | private $refBackend = null; |
||
| 38 | |||
| 39 | /** |
||
| 40 | * Constructor |
||
| 41 | * @param array $serverConfigPrefixes array containing the config Prefixes |
||
| 42 | */ |
||
| 43 | View Code Duplication | public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig) { |
|
| 53 | |||
| 54 | /** |
||
| 55 | * Tries the backends one after the other until a positive result is returned from the specified method |
||
| 56 | * @param string $uid the uid connected to the request |
||
| 57 | * @param string $method the method of the user backend that shall be called |
||
| 58 | * @param array $parameters an array of parameters to be passed |
||
| 59 | * @return mixed the result of the method or false |
||
| 60 | */ |
||
| 61 | protected function walkBackends($uid, $method, $parameters) { |
||
| 76 | |||
| 77 | /** |
||
| 78 | * Asks the backend connected to the server that supposely takes care of the uid from the request. |
||
| 79 | * @param string $uid the uid connected to the request |
||
| 80 | * @param string $method the method of the user backend that shall be called |
||
| 81 | * @param array $parameters an array of parameters to be passed |
||
| 82 | * @param mixed $passOnWhen the result matches this variable |
||
| 83 | * @return mixed the result of the method or false |
||
| 84 | */ |
||
| 85 | protected function callOnLastSeenOn($uid, $method, $parameters, $passOnWhen) { |
||
| 113 | |||
| 114 | /** |
||
| 115 | * Check if backend implements actions |
||
| 116 | * @param int $actions bitwise-or'ed actions |
||
| 117 | * @return boolean |
||
| 118 | * |
||
| 119 | * Returns the supported actions as int to be |
||
| 120 | * compared with OC_USER_BACKEND_CREATE_USER etc. |
||
| 121 | */ |
||
| 122 | public function implementsActions($actions) { |
||
| 126 | |||
| 127 | /** |
||
| 128 | * Backend name to be shown in user management |
||
| 129 | * @return string the name of the backend to be shown |
||
| 130 | */ |
||
| 131 | public function getBackendName() { |
||
| 134 | |||
| 135 | /** |
||
| 136 | * Get a list of all users |
||
| 137 | * |
||
| 138 | * @param string $search |
||
| 139 | * @param null|int $limit |
||
| 140 | * @param null|int $offset |
||
| 141 | * @return string[] an array of all uids |
||
| 142 | */ |
||
| 143 | View Code Duplication | public function getUsers($search = '', $limit = 10, $offset = 0) { |
|
| 154 | |||
| 155 | /** |
||
| 156 | * check if a user exists |
||
| 157 | * @param string $uid the username |
||
| 158 | * @return boolean |
||
| 159 | */ |
||
| 160 | public function userExists($uid) { |
||
| 163 | |||
| 164 | /** |
||
| 165 | * check if a user exists on LDAP |
||
| 166 | * @param string|\OCA\User_LDAP\User\User $user either the ownCloud user |
||
| 167 | * name or an instance of that user |
||
| 168 | * @return boolean |
||
| 169 | */ |
||
| 170 | public function userExistsOnLDAP($user) { |
||
| 174 | |||
| 175 | /** |
||
| 176 | * Check if the password is correct |
||
| 177 | * @param string $uid The username |
||
| 178 | * @param string $password The password |
||
| 179 | * @return bool |
||
| 180 | * |
||
| 181 | * Check if the password is correct without logging in the user |
||
| 182 | */ |
||
| 183 | public function checkPassword($uid, $password) { |
||
| 186 | |||
| 187 | /** |
||
| 188 | * returns the username for the given login name, if available |
||
| 189 | * |
||
| 190 | * @param string $loginName |
||
| 191 | * @return string|false |
||
| 192 | */ |
||
| 193 | public function loginName2UserName($loginName) { |
||
| 197 | |||
| 198 | /** |
||
| 199 | * returns the username for the given LDAP DN, if available |
||
| 200 | * |
||
| 201 | * @param string $dn |
||
| 202 | * @return string|false with the username |
||
| 203 | */ |
||
| 204 | public function dn2UserName($dn) { |
||
| 205 | $id = 'DN,' . $dn; |
||
| 206 | return $this->handleRequest($id, 'dn2UserName', array($dn)); |
||
| 207 | } |
||
| 208 | |||
| 209 | /** |
||
| 210 | * get the user's home directory |
||
| 211 | * @param string $uid the username |
||
| 212 | * @return boolean |
||
| 213 | */ |
||
| 214 | public function getHome($uid) { |
||
| 217 | |||
| 218 | /** |
||
| 219 | * get display name of the user |
||
| 220 | * @param string $uid user ID of the user |
||
| 221 | * @return string display name |
||
| 222 | */ |
||
| 223 | public function getDisplayName($uid) { |
||
| 226 | |||
| 227 | /** |
||
| 228 | * checks whether the user is allowed to change his avatar in ownCloud |
||
| 229 | * @param string $uid the ownCloud user name |
||
| 230 | * @return boolean either the user can or cannot |
||
| 231 | */ |
||
| 232 | public function canChangeAvatar($uid) { |
||
| 235 | |||
| 236 | /** |
||
| 237 | * Get a list of all display names and user ids. |
||
| 238 | * @param string $search |
||
| 239 | * @param string|null $limit |
||
| 240 | * @param string|null $offset |
||
| 241 | * @return array an array of all displayNames (value) and the corresponding uids (key) |
||
| 242 | */ |
||
| 243 | public function getDisplayNames($search = '', $limit = null, $offset = null) { |
||
| 254 | |||
| 255 | /** |
||
| 256 | * delete a user |
||
| 257 | * @param string $uid The username of the user to delete |
||
| 258 | * @return bool |
||
| 259 | * |
||
| 260 | * Deletes a user |
||
| 261 | */ |
||
| 262 | public function deleteUser($uid) { |
||
| 265 | |||
| 266 | /** |
||
| 267 | * Set password |
||
| 268 | * @param string $uid The username |
||
| 269 | * @param string $password The new password |
||
| 270 | * @return bool |
||
| 271 | * |
||
| 272 | */ |
||
| 273 | public function setPassword($uid, $password) { |
||
| 274 | return $this->handleRequest($uid, 'setPassword', array($uid, $password)); |
||
| 275 | } |
||
| 276 | |||
| 277 | /** |
||
| 278 | * @return bool |
||
| 279 | */ |
||
| 280 | public function hasUserListings() { |
||
| 283 | |||
| 284 | /** |
||
| 285 | * Count the number of users |
||
| 286 | * @return int|bool |
||
| 287 | */ |
||
| 288 | public function countUsers() { |
||
| 298 | |||
| 299 | /** |
||
| 300 | * Return access for LDAP interaction. |
||
| 301 | * @param string $uid |
||
| 302 | * @return Access instance of Access for LDAP interaction |
||
| 303 | */ |
||
| 304 | public function getLDAPAccess($uid) { |
||
| 307 | |||
| 308 | /** |
||
| 309 | * Return a new LDAP connection for the specified user. |
||
| 310 | * The connection needs to be closed manually. |
||
| 311 | * @param string $uid |
||
| 312 | * @return resource of the LDAP connection |
||
| 313 | */ |
||
| 314 | public function getNewLDAPConnection($uid) { |
||
| 317 | } |
||
| 318 |
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.