BurningFlipside /
Profiles
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 UsersAPI 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 UsersAPI, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 2 | class UsersAPI extends ProfilesAdminAPI |
||
| 3 | { |
||
| 4 | public function setup($app) |
||
| 18 | |||
| 19 | public function listUsers($request, $response) |
||
| 20 | { |
||
| 21 | $users = false; |
||
|
|
|||
| 22 | $odata = $request->getAttribute('odata', new \ODataParams(array())); |
||
| 23 | if($this->validateIsAdmin($request, true) === false) |
||
| 24 | { |
||
| 25 | $users = array($this->user); |
||
| 26 | $users = $odata->filterArrayPerSelect($users); |
||
| 27 | } |
||
| 28 | else |
||
| 29 | { |
||
| 30 | $auth = AuthProvider::getInstance(); |
||
| 31 | $users = $auth->getUsersByFilter($odata->filter, $odata->select, $odata->top, $odata->skip, |
||
| 32 | $odata->orderby); |
||
| 33 | } |
||
| 34 | return $response->withJson($users); |
||
| 35 | } |
||
| 36 | |||
| 37 | protected function validateCanCreateUser($proposedUser, $auth, &$message) |
||
| 53 | |||
| 54 | protected function validEmail($email) |
||
| 55 | { |
||
| 56 | if(filter_var($email) === false) |
||
| 57 | { |
||
| 58 | return false; |
||
| 59 | } |
||
| 60 | $pos = strpos($email, '@'); |
||
| 61 | if($pos === false) |
||
| 62 | { |
||
| 63 | return false; |
||
| 64 | } |
||
| 65 | $domain = substr($email, $pos + 1); |
||
| 66 | if(checkdnsrr($domain, 'MX') === false) |
||
| 67 | { |
||
| 68 | return false; |
||
| 69 | } |
||
| 70 | return true; |
||
| 71 | } |
||
| 72 | |||
| 73 | public function createUser($request, $response) |
||
| 74 | { |
||
| 75 | $this->user = $request->getAttribute('user'); |
||
| 76 | //This one is different. If they are logged in fail... |
||
| 77 | if($this->user !== false) |
||
| 78 | { |
||
| 79 | return $response->withStatus(404); |
||
| 80 | } |
||
| 81 | $obj = $request->getParsedBody(); |
||
| 82 | if(!isset($obj->captcha)) |
||
| 83 | { |
||
| 84 | return $response->withStatus(401); |
||
| 85 | } |
||
| 86 | $captcha = FlipSession::getVar('captcha'); |
||
| 87 | if($captcha === false) |
||
| 88 | { |
||
| 89 | return $response->withStatus(401); |
||
| 90 | } |
||
| 91 | if(!$captcha->is_answer_right($obj->captcha)) |
||
| 92 | { |
||
| 93 | return $response->withJson(array('res'=>false, 'message'=>'Incorrect answer to CAPTCHA!'), 412); |
||
| 94 | } |
||
| 95 | $auth = AuthProvider::getInstance(); |
||
| 96 | $message = false; |
||
| 97 | if($this->validateCanCreateUser($obj, $auth, $message) === false) |
||
| 98 | { |
||
| 99 | return $response->withJson(array('res'=>false, 'message'=>$message), 412); |
||
| 100 | } |
||
| 101 | View Code Duplication | else if($this->validEmail($obj->mail) === false) |
|
| 102 | { |
||
| 103 | return $response->withJson(array('res'=>false, 'message'=>'Invalid Email Address!')); |
||
| 104 | } |
||
| 105 | $ret = $auth->createPendingUser($obj); |
||
| 106 | View Code Duplication | if($ret == false) |
|
| 107 | { |
||
| 108 | return $response->withJson(array('res'=>false, 'message'=>'Failed to save user registration!'), 500); |
||
| 109 | } |
||
| 110 | return $response->withJson($ret); |
||
| 111 | } |
||
| 112 | |||
| 113 | protected function userIsMe($request, $uid) |
||
| 118 | |||
| 119 | View Code Duplication | protected function getUserByUIDReadOnly($request, $uid) |
|
| 137 | |||
| 138 | View Code Duplication | protected function getUserByUID($request, $uid) |
|
| 156 | |||
| 157 | public function showUser($request, $response, $args) |
||
| 191 | |||
| 192 | protected function sendPasswordResetEmail($user) |
||
| 206 | |||
| 207 | protected function exceptionCodeToHttpCode($e) |
||
| 215 | |||
| 216 | protected function getUser($request, $uid, $payload) |
||
| 231 | |||
| 232 | public function editUser($request, $response, $args) |
||
| 267 | |||
| 268 | public function deleteUser($request, $response, $args) |
||
| 300 | |||
| 301 | public function listGroupsForUser($request, $response, $args) |
||
| 321 | |||
| 322 | public function linkUser($request, $response, $args) |
||
| 351 | |||
| 352 | protected function getAllUsersByFilter($filter, &$pending) |
||
| 369 | |||
| 370 | public function checkEmailAvailable($request, $response) |
||
| 371 | { |
||
| 372 | $params = $request->getQueryParams(); |
||
| 373 | $email = false; |
||
| 374 | if(isset($params['email'])) |
||
| 375 | { |
||
| 376 | $email = $params['email']; |
||
| 377 | } |
||
| 378 | if($email === false) |
||
| 379 | { |
||
| 380 | return $response->withStatus(400); |
||
| 381 | } |
||
| 382 | if(filter_var($email, FILTER_VALIDATE_EMAIL) === false || strpos($email, '@') === false) |
||
| 383 | { |
||
| 384 | return $response->withJson(false); |
||
| 385 | } |
||
| 386 | if(strstr($email, '+') !== false) |
||
| 387 | { |
||
| 388 | //Remove everything between the + and the @ |
||
| 389 | $begining = strpos($email, '+'); |
||
| 390 | $end = strpos($email, '@'); |
||
| 391 | $to_delete = substr($email, $begining, $end - $begining); |
||
| 392 | $email = str_replace($to_delete, '', $email); |
||
| 393 | } |
||
| 394 | $filter = new \Data\Filter('mail eq '.$email); |
||
| 395 | $pending = false; |
||
| 396 | $user = $this->getAllUsersByFilter($filter, $pending); |
||
| 397 | if($user === false) |
||
| 398 | { |
||
| 399 | return $response->withJson(true); |
||
| 400 | } |
||
| 401 | return $response->withJson(array('res'=>false, 'email'=>$user->mail, 'pending'=>$pending)); |
||
| 402 | } |
||
| 403 | |||
| 404 | public function checkUidAvailable($request, $response) |
||
| 405 | { |
||
| 406 | $params = $request->getQueryParams(); |
||
| 407 | $uid = false; |
||
| 408 | if(isset($params['uid'])) |
||
| 409 | { |
||
| 410 | $uid = $params['uid']; |
||
| 411 | } |
||
| 412 | if($uid === false) |
||
| 413 | { |
||
| 414 | return $response->withStatus(400); |
||
| 415 | } |
||
| 416 | if(strpos($uid, '=') !== false || strpos($uid, ',') !== false) |
||
| 417 | { |
||
| 418 | return $response->withJson(false); |
||
| 419 | } |
||
| 420 | $filter = new \Data\Filter('uid eq '.$uid); |
||
| 421 | $pending = false; |
||
| 422 | $user = $this->getAllUsersByFilter($filter, $pending); |
||
| 423 | if($user === false) |
||
| 424 | { |
||
| 425 | return $response->withJson(true); |
||
| 426 | } |
||
| 427 | return $response->withJson(array('res'=>false, 'uidl'=>$user->uid, 'pending'=>$pending)); |
||
| 428 | } |
||
| 429 | |||
| 430 | public function resetUserPassword($request, $response, $args) |
||
| 455 | |||
| 456 | public function remindUid($request, $response) |
||
| 457 | { |
||
| 458 | $params = $request->getQueryParams(); |
||
| 459 | $email = false; |
||
| 460 | if(isset($params['email'])) |
||
| 485 | } |
||
| 486 | /* vim: set tabstop=4 shiftwidth=4 expandtab: */ |
||
| 487 |
Loading history...
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVarassignment in line 1 and the$higherassignment in line 2 are dead. The first because$myVaris never used and the second because$higheris always overwritten for every possible time line.