Multiple condition if else statement not working

Question

if(isset($_POST['submitRegister'])) {

$username = $_POST['username'];
$password = $_POST['password'];
$password2 = $_POST['password2'];
$email = $_POST['email'];

if(!preg_match('#^[a-zA-Z0-9_-]+$#', $username))
    $error1 = 'Username can only contain: A-Z a-z 0-9 _ - ';

if(!isset($username) || empty($username))
    $error1 = 'Please enter your Username';


if(!preg_match("#[0-9]+#", $password))
    $error2 = 'Password must include at least one number';

if(!isset($password) || empty($password))
    $error2 = 'Please enter your Password';     


if($password != $password2)
    $error3 = 'Passwords do not match';

if(!isset($password2) || empty($password2))
    $error3 = 'Please confirm your Password';


if(!preg_match("#^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}+$#", $email))
    $error4 = 'That e-mail does not appear to be valid';

if(!isset($email) || empty($email))
    $error4 = 'Please enter your E-mail address';       


if(!isset($_POST["terms"]))
    $error5 = 'You must accept the Terms and Conditions';

 else {

    require_once 'server.php';
    require_once 'dbconn.php';

}
}

I'm having some trouble in getting this to work correctly. As you can see I have multiple if conditions. In fact there are a few more, but I've edited it to make the post a bit shorter. I've gone through lots of if else tuts and thought I understood it, but clearly not.

My problem is, as long as I don't tick the terms checkbox (the last if statement), everything works fine. But if I tick the checkbox, it will attempt to connect to the database, regardless if the previous fields are empty or not filled in correctly.

Thinking upon my last post where I had put my ifs in the wrong order (back to front), I placed the if statement for the terms first and the username one last. I thought this solved it, but so long as I put in a username it would connect, regardless if the other fields were empty. So that did not work in this case. Hoping someone can help, many thanks.

Answer 1

if(!isset($_POST["terms"]))
    $error5 = 'You must accept the Terms and Conditions';

 else {

    require_once 'server.php';
    require_once 'dbconn.php';
}

if ... else blocks work in pairs, none of the if statement preceding this one have any effect on the else condition. Essentially, those require_once calls will happen if $_POST['terms'] is set - irrespective of anything else.

I think what you want is something more like:

if(isset($_POST['submitRegister'])) {

    $username = $_POST['username'];
    $password = $_POST['password'];
    $password2 = $_POST['password2'];
    $email = $_POST['email'];

    //define errors, I'm going to put them into an array 
    // - it makes it easier to evaluate at the end
    $aErrors = array();

    if(!preg_match('#^[a-zA-Z0-9_-]+$#', $username)) {
        $aErrors['error_1'] = 'Username can only contain: A-Z a-z 0-9 _ - ';
    }
    elseif(!isset($username) || empty($username)) {
        $aErrors['error_1'] = 'Please enter your Username';
    }

    if(!preg_match("#[0-9]+#", $password)) {
        $aErrors['error_2'] = 'Password must include at least one number';
    }
    elseif(!isset($password) || empty($password)) {
        $aErrors['error_2'] = 'Please enter your Password';     
    }

    if($password != $password2) {
        $aErrors['error_3'] = 'Passwords do not match';
    }
    elseif(!isset($password2) || empty($password2)) {
        $aErrors['error_3'] = 'Please confirm your Password';
    }

    if(!preg_match("#^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}+$#", $email)) {
        $aErrors['error_4'] = 'That e-mail does not appear to be valid';
    }
    elseif(!isset($email) || empty($email)) {
        $aErrors['error_4'] = 'Please enter your E-mail address';
    }

    if(!isset($_POST["terms"])) {
        $aErrors['error_5'] = 'You must accept the Terms and Conditions';
    }

    //if there are no errors
    if(!$aErrors) {
        require_once 'server.php';
        require_once 'dbconn.php';
    }
}

Answer 2

Sorry I don't understand your intention very well, but I can tell you that every time you check the terms checkbox, the next code:

 require_once 'server.php';
 require_once 'dbconn.php';

will be executed.

If you want that require_once statements being only executed when there aren't errors you can do it like this:

if (!$error1 && !$error2 && !$error3 && !$error4 && !$error5)
{
    require_once 'server.php';
    require_once 'dbconn.php';
}

I suggest you to use {} block in all your if statements.

Answer 3

Right, so let's take a step back and look at how if statements work.

in pseudo-code:

if(statement){
    code1
}

code2

Now, if statement is true, then code1 will execute, and code2 will execute. if statement is false, then code1 will not execute, and code2 will execute regardless, because it is not being contained by the if statement block. So applying this to your code, if we have:

if(!isset($email) || empty($email))
    $error4 = 'Please enter your E-mail address';

The way PHP works, is it allows you to have one line without requiring curly brackets, but you can assume it's putting it there so that it would look like this:

if(!isset($email) || empty($email)){
    $error4 = 'Please enter your E-mail address';
}

So then the problem arises. regardless of the outcome of that if statement, all code AFTER it is still going to execute. So what do?

Well, you could do a large nested if statement

if(statement1){
    if(statement2){
        if(statement3){
            ...
        }else{
            error
        }
    }else{
        error
    }
}else{
    error
}

But you can see how ugly this becomes quickly.

So we can do one if statement with multiple conditions,

if(statement1 and statement2 and statement3 and ...){
    code
}else{
    error
}

but with a lot of statements, like yours, that also becomes ugly quickly. So what do? use elseif! elseif only executes if the previous if statement was not executed.

if(!statement1){
    error
}elseif(!statement2){
    error
}elseif(!statement3){
    error
}else{ //meaning no error was triggered
    code
}

Another solution: in most languages, there's a couple functions, try and catch, these prove to be useful. watch:

try{
    if(!statement1){
        throw exception
    }
    if(!statement2){
        throw exception
    }
    if(!statement3){
        throw exception
    }
    
    code
}catch(exception){
    throw error because <exception>
}

Why is this helpful? if an exception is thrown, the code in the try block stops executing and immediately goes to the catch block. Which means all the rest of your code doesn't get executed (so in your case, those errors you want to call, will actually be called).

So, my solution? throw your code into some try - catch blocks

Answer 4

The problem is that the else statement only apply for the last if (whether the conditions are checked or not).

I'd suggest you to do something like :

if(isset($_POST['submitRegister'])) {

    $username = $_POST['username'];
    $password = $_POST['password'];
    $password2 = $_POST['password2'];
    $email = $_POST['email'];
    $valid = false;

    if(!preg_match('#^[a-zA-Z0-9_-]+$#', $username))
        $error1 = 'Username can only contain: A-Z a-z 0-9 _ - ';

    if(!isset($username) || empty($username))
        $error1 = 'Please enter your Username';


    if(!preg_match("#[0-9]+#", $password))
        $error2 = 'Password must include at least one number';

    if(!isset($password) || empty($password))
        $error2 = 'Please enter your Password';     


    if($password != $password2)
        $error3 = 'Passwords do not match';

    if(!isset($password2) || empty($password2))
        $error3 = 'Please confirm your Password';


    if(!preg_match("#^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}+$#", $email))
        $error4 = 'That e-mail does not appear to be valid';

    if(!isset($email) || empty($email))
        $error4 = 'Please enter your E-mail address';       


    if(!isset($_POST["terms"]))
        $error5 = 'You must accept the Terms and Conditions';

    $valid = !(isset($error1) || isset($error2) || ...); //The number of possible errors

    if($valid)
     {

        require_once 'server.php';
        require_once 'dbconn.php';

    }
}

Please note that another way to save the errors would be to store them in an array.

$errors = array();

if(/* Invalid username */)
    $errors[] = 'Invalid username'; //Add new error to the list

/*
...
*/

$valid = count($errors) === 0; //Valid if there is no error

Answer 5

You don't need the "else" in this case last case. Multiple of the "if" statements here can match. The very last one (the "terms" one) could match, and if and only if it doesn't match, the "else" block is executed. You're "else" block is the only part that calls your require_once.

If you remove the else, it should just start working.