A little refactoringIn 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 ^^