snippetphpMinor
PHP - create a new user into a db using MVC framework
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
Here is my model for the user - Model_User.php:
Here is the key snippet from my user registration controller, Controller_Register.php
Finally, here is my user DAO code, with the key item I would like guidance on, the registration - DAO_User.php
```
dbI
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
By
In yout Controller :
Remove the
My way to write this code :
If there is an error, it's not necessary to retry immediatly. Errors won't miraculously fix themselves !
Your DAO is very complicated.
Remove all
You can use two private methods to create and set the password.
Generally, yout code is too much commented for me.
Exemple :
Don't write obvious comment :
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 ^^
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 uselessDon't write obvious comment :
if ($insertedUser){ // User was correctly inserted in dbDon'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 uselessif ($insertedUser){ // User was correctly inserted in dbContext
StackExchange Code Review Q#2877, answer score: 2
Revisions (0)
No revisions yet.