[SOLVED] more efficient way to code this?

[jesushax]

hi all, this is my error checking system for registrations

 

it grabs all the emails and all the postcodes from the db then compares the domains and the postcodes to see if any of them match to what has just been inputted

 

is there a better way to do the below?

 

list($name, $domain) = explode("@",$_POST["Sect1_6"]);
$postcode = trim(strtoupper($_POST["Sect1_7d"]));

$sql = mysql_query("SELECT `Sect1_6`,`Sect1_7d`,`CompanyID` FROM tblDirectory2") or die(mysql_error());
while($row = mysql_fetch_array($sql)) {

list($namecheck, $domaincheck) = explode("@", $row["Sect1_6"]);
$postcodecheck = trim(strtoupper($row["Sect1_7d"]));

if ($domaincheck == $domain ) {
header("Location: /new_directory/completed.php?ID=".$row["CompanyID"]);
exit();}

if ($postcodecheck == $postcode) {
header("Location: /new_directory/completed.php?ID=".$row["CompanyID"]);
exit();}

}

[Yesideez]

Looks fine to me, only thing I'd suggest is indenting to make it more readable and to help make maintenance in the future a lot easier:

list($name, $domain) = explode("@",$_POST["Sect1_6"]);
$postcode = trim(strtoupper($_POST["Sect1_7d"]));

$sql = mysql_query("SELECT `Sect1_6`,`Sect1_7d`,`CompanyID` FROM tblDirectory2") or die(mysql_error());
while($row = mysql_fetch_array($sql)) {

  list($namecheck, $domaincheck) = explode("@", $row["Sect1_6"]);
  $postcodecheck = trim(strtoupper($row["Sect1_7d"]));

  if ($domaincheck == $domain ) {
    header("Location: /new_directory/completed.php?ID=".$row["CompanyID"]);
    exit();
  }

  if ($postcodecheck == $postcode) {
    header("Location: /new_directory/completed.php?ID=".$row["CompanyID"]);
    exit();
  }

}

[Cep]

To make it ever so slightly (so that you won't even notice) efficient you should change these lines

 

header("Location: /new_directory/completed.php?ID=".$row["CompanyID"]);

 

to

 

header("Location: /new_directory/completed.php?ID={$row["CompanyID"]}");

 

The reason being interpretation is faster then concatenation. Other then that I don't see anything wrong either, except your not escaping your POST vars.

[jesushax]

what you mean escaping post vars?

 

thanks

[Yesideez]

It's not escaping - can't remember what it's called but surrounding variables in braces.

 

I prefer to use the method you've already used as when editing code in an editor that color codes everything variables stand out. Including them inside strings like that sort of hides them from view a bit.

 

For the speed increase it isn't worth it unless your site is massive, gets LOTS of hits and is run many times like inside a loop.

[Cep]

Your accepting your POST vars without first validating that the data being passed to your script is not malicious. What if I injected some nice little script into yours via your POST var, that would not be very good.

 

Escaping is probably the wrong term to use with PHP but basically you should always html_special_chars() your POST vars or use a similar function. You take a performance hit but when it comes to security of an app over performance never take the later as the more important.