Coding Style For Nested Ifs

[zscipio]

When the user must enter a lot of data correctly before the data can be stored the code usually looks like this with nested ifs and lots of braces at the end.
[php]
if ( empty ($_POST[‘id’])) // Check for the required value.
{
printMessage(“Please enter a logid!”);
}
else
{
$id = $_POST[‘id’];
if ( empty ($_POST[‘pass’])) // Check for the required value.
{
printMessage(“Please enter a password!”);
}
else
{
$pass = $_POST[‘pass’];
if ( $_POST[‘pass’] != $_POST[‘vpass’]) // Do the passwords match
{
printMessage(“The passwords do not match!”);
}
else
{
$vpass = $_POST[‘vpass’];
if ( empty ($_POST[‘email’])) // Check for the required value.
{
printMessage(“Please enter an email!”);
}
else
{
$email = $_POST[‘email’];
if (!filter_var($email, FILTER_VALIDATE_EMAIL))
{
printMessage(“Please enter an valid email!”);
}
else
{
$itExists = true;
$itExists = loginExists($id, $host );
if ($itExists)
{
printMessage(“This UserID is already registered!”);
}
else
{
$lastChangeDate = time();
$query = “INSERT INTO Customerinfo_cui (username_key_cui, password_cui, email_cui, date_lastchange_cui)
VALUES ( ‘$id’, ‘$pass’, ‘$email’ , $lastChangeDate)”;
require (‘r_connectToDB.htm’);
if (!mysql_query($query,$con))
{
die('
Error insert: ’ . mysql_error());
}
else
{
printMessage(“You are now registered. Please go to Home Page!”);
gotoAPage(null);

                                $_SESSION['username'] = $id;
                                $_SESSION['password'] = $pass;
                                $_SESSION['email']     = $email   ;
                                $_SESSION['lastChangeDate']     = $lastChangeDate;
                             }
                          }
                       }
                    }
                 }
              }
           }

[/php]

I don’t like the indenting further & further so I came up with the following substitution.

[php]

            do
            {
               if ( empty ($_POST['id'])) // Check for the required value.
               {   
                  printMessage("Please enter a logid!");
                  break;
               }
               $id = $_POST['id'];
               
               if ( empty ($_POST['pass'])) // Check for the required value.
               {   
                  printMessage("Please enter a password!");
                  break;
               }
               $pass = $_POST['pass'];
               
               if ( $_POST['pass'] != $_POST['vpass']) // Do the passwords match
               {   
                  printMessage("The passwords do not match!");
                  break;
               }
               $vpass = $_POST['vpass'];
               
               if ( empty ($_POST['email'])) // Check for the required value.
               {   
                  printMessage("Please enter an email!");
                  break;
               }
               $email = $_POST['email'];
               
               if (!filter_var($email, FILTER_VALIDATE_EMAIL))
               {
                  printMessage("Please enter an valid email!");
                  break;
               }
               $itExists = true;
               $itExists = loginExists($id, $host );
               if ($itExists)
               {
                  printMessage("This UserID is already registered!");
                  break;
               }
               $lastChangeDate = time();
               
               $query = "INSERT INTO Customerinfo_cui (username_key_cui, password_cui, email_cui, date_lastchange_cui)
                            VALUES (   '$id',     '$pass',     '$email' , $lastChangeDate)";
               require ('r_connectToDB.htm');                  
               if (!mysql_query($query,$con))
               {
                  die('<br>Error insert: ' . mysql_error());
               }
               printMessage("You are now registered. Please go to Home Page!");
               gotoAPage(null);
            
               $_SESSION['username'] = $id;
               $_SESSION['password'] = $pass;
               $_SESSION['email']     = $email   ;
               $_SESSION['lastChangeDate']     = $lastChangeDate   ;
            }while (false);

[/php]

The working code is at the end and no indentation & dangling }. Please comment

[aaronszy]

Presumably this code belongs to a function. If not, you should probably put it in one. If you want to do it all in a single function, you could do something like the following:

NOTE: This example is cleaner, but really insecure. Make sure you sanitize all your SQL parameters before you put this code on a production server.

[php]
function registerUser()
{
/*
* Validate user input
*/

// Check for the required value.
if(empty($_POST[‘id’]))
{
printMessage(“Please enter a logid!”);
return;
}
$id = $_POST[‘id’];

// Check for the required value.
if(empty($_POST[‘pass’]))
{
printMessage(“Please enter a password!”);
return;
}
$pass = $_POST[‘pass’];

// Do the passwords match?
if($_POST[‘pass’] != $_POST[‘vpass’])
{
printMessage(“The passwords do not match!”);
return;
}
$vpass = $_POST[‘vpass’];

// Check for the required value
if(empty($_POST[‘email’]))
{
printMessage(“Please enter an email!”);
return;
}
$email = $_POST[‘email’];

// Valid email address?
if(!filter_var($email, FILTER_VALIDATE_EMAIL))
{
printMessage(“Please enter an valid email!”);
return;
}

if(loginExists($id, $host))
{
printMessage(“This UserID is already registered!”);
return;
}

/*
* Register the user
*/

$lastChangeDate = time();
$query = “INSERT INTO Customerinfo_cui (username_key_cui, password_cui, email_cui, date_lastchange_cui) VALUES (’$id’, ‘$pass’, ‘$email’, $lastChangeDate)”;
require(‘r_connectToDB.htm’);
if(!mysql_query($query,$con))
{
die('
Error insert: ’ . mysql_error());
}

printMessage(“You are now registered. Please go to Home Page!”);
gotoAPage(null);

$_SESSION[‘username’] = $id;
$_SESSION[‘password’] = $pass;
$_SESSION[‘email’] = $email;
$_SESSION[‘lastChangeDate’] = $lastChangeDate;
}
[/php]

However, generally if you have really deep nesting, it’s a sign that you’re trying to make your function do too much work. If you break out the nested control blocks into their own functions you will end up with cleaner, easier to understand code.

As for using the do/while with breaks, its a clever idea, but a little wonky. Probably something to avoid if you want to keep your code easy to read.

Good luck!

[zscipio]

I have been coding since 1959 in procedural code (PC) and in object oriented (OO) for the last 5 years so I think in PC but translate & write in OO. Sort of like thinking in French & speaking & writing English. So my code will come out quirky, but I am trying to think in OO.

The code was not in a function but I could put each validation in a separate function.

What do you mean by “NOTE: This example is cleaner, but really insecure.”?

What do you mean by “Make sure you sanitize all your SQL parameters before you put this code on a production server.”?

Thank you

[aaronszy]

Well, what would happen if someone tried to set their email to something like:
“’);drop table Customerinfo_cui;–”

That’s just an example, but people can get pretty creative. So make sure you run all your parameters through: mysql_real_escape_string() before contatenating them into the query.

[php]
$cleanEmail = mysql_real_escape_string($email);
$query = “INSERT INTO Customerinfo_cui (username_key_cui, password_cui, email_cui, date_lastchange_cui) VALUES (’$id’, ‘$pass’, ‘$cleanEmail’, $lastChangeDate)”;
[/php]