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

Validating username fields and showing CAPTCHA

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Why not use the built-in PHP e-mail validation?

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.