2
\$\begingroup\$

When a user is created, deleted and update the system will give feed back. e.g. User Deleted/Updated/Create success or unsuccessfully, but I'm sure there is a better way to do it than what I have done because I repeat the same code in all functions within my class.

Code to create user

$userOne = new userActions($database, 'Greg', '324b2643243');//
echo $userOne->create();

Code to delete user

$userOne = new userActions($database, 'Abel'); //
echo $userOne->delete();

Code for user creation

public function create()
{//Works
    $outPut = "";
    if(!$this->user_exists)
        {
            list($password_hash, $salt) = $this->hash($this->password);
            echo 'Hash: ' . $password_hash . '<br>' . 'Salt: ' . $salt . '<br />';
            $result = $this->connection->query("INSERT INTO users VALUES( NULL, '{$this->username}' , '{$password_hash}' , '{$salt}' ) ");

            if($result)
                {
                    $outPut = "User has been created";
                }
                else
                {
                    $outPut = "User could not be created";
                }

        }
        else
        {
            $outPut = 'Username already exists';
        }

    return $outPut;
}

Is there a better way to display feedback to the user istead of having $outPut every time in all of my create, delete and update functions?

\$\endgroup\$
  • \$\begingroup\$ You could make them static functions. echo userActions::create($database, 'Greg', '324b2643243'); If you don't use them for something else. \$\endgroup\$ – AlucardTheRipper Apr 14 '13 at 11:40
1
\$\begingroup\$

I recommend splitting your model (the database operation) from your view (the localization of your message).

Depending if you prefer a return code or a exception flow your code could look like:

public function create()
{
    if($this->user_exists) return ""; //do you really want to show nothing?
    $error=UserModel::create($this->password,$salt,$this->username); // don't have to be static! It's up to you.
    if ($error==-1) return "User could not be created";  //could be a constant UserModel::ERROR_GENERAL
    if ($error==0) return "Username already exists"; //UserModel::ERROR_EXISTS
    return "User has been created";
}

Translating this to exception is up to the reader :)

Actually I even would prefer to give the $error code to my template and let it handle the localization:

public function create()
{
    if($this->user_exists) return "";
    $error=UserModel::create($this->password,$salt,$this->username); 
    $view=new View("templatefile.php");
    $view->assign('error',$error);
    return $view->render();
}

//"templatefile.php"
<?if ($error==1):?>...
<?elseif ($error==0):?>...
<?else:?>...
<?endif?>
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy