Tutorial :What would you change in my code for best practices/maintenance?



Question:

I've got a small snippet of code below and I was curious what types of things you would change with regards to best practices/code maintainablity and so on.

function _setAccountStatus($Username, $AccountStatus)  {  if ($Username == '' || ($AccountStatus != 'Active' || $AccountStatus != 'Banned' || $AccountStatus != 'Suspended')) {      // TODO: throw error here.  }    $c1 = new Criteria();    $c1->add(UsersPeer::USERNAME,$Username);    $rs = UsersPeer::doSelect($c1);    if (count($rs) > 0) {      $UserRow = array_pop($rs);        $UserRow->setAccountStatus($AccountStatus);        try {          $UserRow->save();      } catch ( PropelException $e ) {          return false;      }        return true;  }    return false;  }  


Solution:1

I would use the empty() instead of $Username == '' in your if statement. I haven't used propel before, but I would prefer to have this method be on my User object itself with the fetching and saving of the user object performed by a seperate object. Pseudo code would be something like this.

$user = userManager->getUser($username); $user->setAccountStatus($accountStatus); $userManager->saveUser($user);


Solution:2

An else clause before the last return false would be prefererred, just to make the code more readable.


Note:If u also have question or solution just comment us below or mail us on toontricks1994@gmail.com
Previous
Next Post »