patternphpMinor
Validating username fields and showing CAPTCHA
Viewed 0 times
captchashowingfieldsvalidatingusernameand
Problem
I'm new to coding, and especially to OOP PHP. I'll highly appreciate if you expert programmers could review following code and give me your opinion to make it better. Also, how you see this code, is it too horrible and ugly?
```
mysqli = $mysqli;
}
function valid_length($input){
if ((strlen($input) > 4 ) AND (strlen ($input) mysqli->prepare("SELECT username FROM users WHERE username=?")) {
$stmt->bind_param("s", $username);
$stmt->execute();
$stmt->store_result();
$count=$stmt->num_rows;
$stmt->close();
}
return ($count > 0 ? true : false);
}
function signup($username, $password, $email, $capt_error){
//validate username.
if(!$this->valid_length($username)){
$this->signup_error = "Username should be 5 - 15 characters. ";
}else if(!$this->alpha_numeric($username)){
$this -> signup_error ="Only letters, numbers";
} else if($this->username_unallowed($username)){
$this -> signup_error ="Not allowed.";
} else if ($this->username_exists($username)){
$this -> signup_error ="Username already exists. ";
}
//validate password.
if (!$this->valid_length($password) ){
$this -> signup_error .="Password should be 5 - 15 characters. ";
}
//validate email.
if (!$this->valid_email($email) ){
$this -> signup_error .="Invalid email.";
}else if($this->email_exists($email)) {
$this -> signup_error .="Email exists.";
}
//captcha error
if($capt_error == 1){
$this -> signup_error .="Invalid captcha. ";
}
//no any error, proceed.
if ($this->signup_error == ""){
$hashed_password = $this->hash_password($password);
if ($this->insert_new_user($username, $enc_password, $email)){
$_SESSION['username']=$username;
header("Location: welcome.php");
}else {
$this
```
mysqli = $mysqli;
}
function valid_length($input){
if ((strlen($input) > 4 ) AND (strlen ($input) mysqli->prepare("SELECT username FROM users WHERE username=?")) {
$stmt->bind_param("s", $username);
$stmt->execute();
$stmt->store_result();
$count=$stmt->num_rows;
$stmt->close();
}
return ($count > 0 ? true : false);
}
function signup($username, $password, $email, $capt_error){
//validate username.
if(!$this->valid_length($username)){
$this->signup_error = "Username should be 5 - 15 characters. ";
}else if(!$this->alpha_numeric($username)){
$this -> signup_error ="Only letters, numbers";
} else if($this->username_unallowed($username)){
$this -> signup_error ="Not allowed.";
} else if ($this->username_exists($username)){
$this -> signup_error ="Username already exists. ";
}
//validate password.
if (!$this->valid_length($password) ){
$this -> signup_error .="Password should be 5 - 15 characters. ";
}
//validate email.
if (!$this->valid_email($email) ){
$this -> signup_error .="Invalid email.";
}else if($this->email_exists($email)) {
$this -> signup_error .="Email exists.";
}
//captcha error
if($capt_error == 1){
$this -> signup_error .="Invalid captcha. ";
}
//no any error, proceed.
if ($this->signup_error == ""){
$hashed_password = $this->hash_password($password);
if ($this->insert_new_user($username, $enc_password, $email)){
$_SESSION['username']=$username;
header("Location: welcome.php");
}else {
$this
Solution
Why not use the built-in PHP e-mail validation?
Why do
when you can do
Same goes for
Furthermore the signup_error stuff is ugly. I'd suggest exceptions. At least make signup_error into an array instead, and just add new elements when you get an error. You can join() the array when outputting.
Also, you risk $count not being defined if $stmt fails.
filter_var($email, FILTER_VALIDATE_EMAIL)Why do
if (lol())
return true;
else
return false;when you can do
return lol();Same goes for
lol() ? true : false which you do too.Furthermore the signup_error stuff is ugly. I'd suggest exceptions. At least make signup_error into an array instead, and just add new elements when you get an error. You can join() the array when outputting.
Also, you risk $count not being defined if $stmt fails.
Code Snippets
filter_var($email, FILTER_VALIDATE_EMAIL)if (lol())
return true;
else
return false;return lol();Context
StackExchange Code Review Q#2624, answer score: 7
Revisions (0)
No revisions yet.