CODEWITHSUNDEEP
phpif-statement
BrokenCode
BrokenCode

Reputation: 961

Multiple if-statements in php?

I have several variables and I would like to validate them before writing them into the mysql database. Basically I would like the following logic: if the username is ok, and the email is ok and the password is ok then write all to the db, else spit out an error for each.

I have tried to come up with the following logic but there must be a more logical and efficient way to do it (sorry, I know this looks primitive):

if ( preg_match("/^[A-Za-z0-9]+(?:[ _-][A-Za-z0-9]+)*$/", $username) ) {
    if ((preg_match("^([0-9a-zA-Z]+[-._+&])*[0-9a-zA-Z]+@([-0-9a-zA-Z]+[.])+[a-zA-Z]{2,6}$", $email)) {
        if (/*password is ok*/) {
            //create user entry in the db
            //and save success message in the session variable
        } else {
            //error message for password
        }
    } else {
        //error message for email
    }
} else {
    //error message for username
}

Upvotes: 0

Views: 232

Answers (4)

Joel
Joel

Reputation: 577

You could use die to end the script

if (/*User name in NOT ok*/)
{
die("Bad username");
}

if(/*Password is not ok*/)
{
die("Bad Pass");
}

Or use variables

$usernameOk = false;
$passwordOk = false;

if(/*Username is ok*/)
{
$username = true;
}

if(/*Password is ok*/)
{
$passwordOk = true;
}

if(/*username true and password true*/)
{
//Execute statement
}

Upvotes: 0

kba
kba

Reputation: 19466

First of all, there are lots of frameworks and validation scripts that allow you to do this. On the other hand, it's good to have done this yourself, so you know what is actually going on, should you later decide to switch to a framework.

Your code looks decent, but from a usability point-of-view, it isn't as good as one could have hoped. If I try to create a user in your system, I might have to fill out the forms and press submit several times, which can be quite annoying. Instead, once the form has been filled out, a list of errors should show up, instead of just one error at a time. A common way to handle this problem is by creating an array of error messages.

An implementation could look something like this

$errors = array();

if (!preg_match("/^[A-Za-z0-9]+(?:[ _-][A-Za-z0-9]+)*$/", $username))
    array_push($errors, "Invalid username");

if (!preg_match("^([0-9a-zA-Z]+[-._+&])*[0-9a-zA-Z]+@([-0-9a-zA-Z]+[.])+[a-zA-Z]{2,6}$", $email))
    array_push($errors, "Invalid password");

if (otherValidationRule())
    array_push($errors, "Something else went wrong");

After this, you check whether any errors occured and print them if so

if (sizeof($errors) > 0) {
    print("<ul>\n");
    foreach($errors as $error)
        printf("<li>%s</li>\n", $error);
    print("</ul>\n");
}
else {
    // Proceed with registration
}

Upvotes: 1

p.g.l.hall
p.g.l.hall

Reputation: 1961

Ideally, you'd want to be able to show multiple error messages, in case there was more than one thing wrong. Something like this:

$errors = array();
if( !preg_match("/^[A-Za-z0-9]+(?:[ _-][A-Za-z0-9]+)*$/", $username) )
{
    $errors[] = 'Username was invalid';
}
if ((preg_match("^([0-9a-zA-Z]+[-._+&])*[0-9a-zA-Z]+@([-0-9a-zA-Z]+[.])+[a-zA-Z]{2,6}$", $email))
{
    $errors[] = 'Email was invalid';
}
if( /* Password is bad */ )
{
    $errors[] = 'Password is bad somehow';
}

if( sizeof($errors) == 0 )
{
    // write to db
}
else
{
    // Display all error messages
}

Upvotes: 4

Travesty3
Travesty3

Reputation: 14469

I think a shorter and more readable way would be:

if (!preg_match("/^[A-Za-z0-9]+(?:[ _-][A-Za-z0-9]+)*$/", $username))
    die("username error");

if (!preg_match("^([0-9a-zA-Z]+[-._+&])*[0-9a-zA-Z]+@([-0-9a-zA-Z]+[.])+[a-zA-Z]{2,6}$", $email))
    die("email error");

if (/*password is NOT ok*/)
    die("password error");

//create user entry in the db
//and save success message in the session variable

Notice that I have negated your conditionals so that the contents of the if-statement only execute if a failure occurs.

Upvotes: 0

Related Questions