HiveBrain v1.2.0
Get Started
← Back to all entries
snippetphpMinor

PHP - create a new user into a db using MVC framework

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
newcreatemvcphpintouserusingframework

Problem

I'm working on the registration process for new users of a small web app, and as this is largely a learning project I'm doing it from scratch.

Here is my user class: Class_user.php

userID=$userID;
            $this->userName=$userName;
            $this->hashedPassword=$hashedPassword;
            $this->userEmail=$userEmail;
        }
        function getUserID(){
            return $this->userID;
        }
        function getUserName() {
            return $this->userName;
        }
        function getHashedPassword() {
            return $this->hashedPassword;
        }
        function getUserEmail() {
            return $this->userEmail;
        }
    }
?>


Here is my model for the user - Model_User.php:

dbInstance=$dbInstance;
        }
        function insertNewUser($user, $userPassword){ // INCOMPLETE
            $userDAO=new DAO_User($this->dbInstance);
            $insertedUser=$userDAO->createNewUser($user, $userPassword);
            $userPassword=""; // We clear the user's password from memory.
            if ($insertedUser){ // User was correctly inserted in db
                return true; // Should return the $user object.
            } else {
                return false;
            }
        }
    }
?>


Here is the key snippet from my user registration controller, Controller_Register.php

insertNewUser($user,$userPassword); // We insert the user name not knowing what the autoincremented user ID is.
                $userPassword=""; // We don't keep the pw in memory.
                }
            $_POST["password"]=""; // We clear the user's password from memory.
            if ($userInserted){// The value is true if the registration was succesful.
            $msg=new Message("Congratulations ".$_POST['userName']."! You are now registered."); 
            }
        }
    return $view;
    }
?>


Finally, here is my user DAO code, with the key item I would like guidance on, the registration - DAO_User.php

```
dbI

Solution

In your User class : add the setters !

You cannot modify a value from outside the class (ex: $user->userID = $userId)

Try it : http://ideone.com/We2Lh

At the end of Model_User.php, replace

if ($insertedUser){ // User was correctly inserted in db
     return true; // Should return the $user object.
} else {
    return false;
}


By

return $insertedUser;


In yout Controller :

Remove the While (!$userInserted) {. What will appened if the insertNewUser method always fail ? an infinite loop, an maybe you can crash your server.

My way to write this code :

function execute($view) {

    // check and sanitize values
    $user = new user($userName, $userEmail); // Create a specific constructor. Empty values are ...
    $userInserted=$userDBConnection->insertNewUser($user,$userPassword);

    if ($userInserted) {
        $msg = new Message("Congratulations ".$_POST['userName']."! You are now registered.");
    } else {
        $msg = new Message("Erreur during subscription. Please retry.");
    }

    // send $msg to the view
    // ...
}


If there is an error, it's not necessary to retry immediatly. Errors won't miraculously fix themselves !

$_POST["password"]=""; : Useless, the parameters will be reset at the next request.

Your DAO is very complicated.

Remove all while, and handle possible errors (you can use boolean, exceptions ...).

You can use two private methods to create and set the password.

Generally, yout code is too much commented for me.

Exemple :

private $userName; // Must be unique.
=> Check this in your DB, a comment is useless


Don't write obvious comment :

if ($insertedUser){ // User was correctly inserted in db


Don't try to explain the How or the What, comment the Why. The 'How' and the 'What' could be understand by reading your code, but you can't undestand the objective of a snippet if it's complicated.

Add an unique index to userName.

Finally, think of aerating your code ! New lines are free ^^

Code Snippets

if ($insertedUser){ // User was correctly inserted in db
     return true; // Should return the $user object.
} else {
    return false;
}
return $insertedUser;
function execute($view) {

    // check and sanitize values
    $user = new user($userName, $userEmail); // Create a specific constructor. Empty values are ...
    $userInserted=$userDBConnection->insertNewUser($user,$userPassword);

    if ($userInserted) {
        $msg = new Message("Congratulations ".$_POST['userName']."! You are now registered.");
    } else {
        $msg = new Message("Erreur during subscription. Please retry.");
    }

    // send $msg to the view
    // ...
}
private $userName; // Must be unique.
=> Check this in your DB, a comment is useless
if ($insertedUser){ // User was correctly inserted in db

Context

StackExchange Code Review Q#2877, answer score: 2

Revisions (0)

No revisions yet.