Not running rest of the code if theres 0 matches

PHP
1

Hi Guys,

I need some help please, i’ve made a script that does a few fraud checks on data submitted through our system (I’m still working on the fraud check script in case some of you think is that the only fraud checks im making). The problem i have is if theres no matches after running the IP Country Check section then the Email Valid Check will not be ran and i dont understand why, if anyone could help me that would be great please.


<?php

require("./databaseconnection.php"); //Get Database Login Information

//Do IP Country Check
mysql_connect(localhost,$username,$password);
@mysql_select_db($database) or die( "Oops theres an error, our highly trained monkeys have been notified.");
$query = "SELECT * FROM `leads` WHERE `CountryCheck` = 'NO'";		
$result=mysql_query($query);
$num=mysql_numrows($result) or die(mysql_error());
$i=0;
while ($i < $num) {
	while($row = mysql_fetch_array($result))
	{
	  $ipcountry=$row['IPCountry'];
	  $ipcc=$row['IPCC'];
	  $ipisp=$row['IPISP'];
	  $leadid=$row['LeadID'];
	  $ProgramID=$row['ProgramID'];
	
	  //Get Country Of Each Lead
		$query2 = "SELECT * FROM `programs` WHERE `ID` = '$ProgramID'";
		$result2=mysql_query($query2);
		while($row = mysql_fetch_array($result2))
		{
			$programcountry=$row['Country'];
		}
		
	  //End
	
		if ( $ipcc == $programcountry ) {
			$query3 = "UPDATE leads SET CountryCheck='PASS' WHERE LeadID='$leadid'";
			mysql_query($query3);
		} else {
				$query3 = "UPDATE leads SET CountryCheck='FAIL' WHERE LeadID='$leadid'";
				mysql_query($query3);
				}
	}		
$i++;
}
mysql_close();
//End
echo "Finished Country Check! <br />";

//Do Email Valid Check
include("./emailcheck.php");
mysql_connect(localhost,$username,$password);
@mysql_select_db($database) or die( "Oops theres an error, our highly trained monkeys have been notified.");
$query = "SELECT * FROM `leads` WHERE `EmailCheck` = 'NO'";		
$result=mysql_query($query);
$num=mysql_numrows($result) or die(mysql_error());
$i=0;
while ($i < $num) {
	while($row = mysql_fetch_array($result))
	{
	  $emailaddress=$row['EmailAddress'];
	  $leadid=$row['LeadID'];
	
	  $bIsEmailValid = jValidateEmailUsingSMTP("$emailaddress", "REMOVED.com", "fraudcheck@REMOVED.com");

		$emailchecker = $bIsEmailValid ? "Valid" : "Invalid";
		
		if ($emailchecker=="Valid")
		  {
		  $query3 = "UPDATE leads SET EmailCheck='PASS' WHERE LeadID='$leadid'";
		  mysql_query($query3);
		  }
		else
		  {
		  $query3 = "UPDATE leads SET EmailCheck='FAIL' WHERE LeadID='$leadid'";
		  mysql_query($query3);
		  }
	
	}		
$i++;
}
mysql_close();
//End

echo "Finished Email Check!";

?>

Any help wouldbe great, thank you in advance!

2

I don’t understand very well what you are saying. Doesn’t the email valid check run when there are no rows with EmailCheck = ‘NO’ ? But that’s what you want, isn’t it?
Or doesn’t the email valid check run when there are no rows with CountryCheck = ‘No’ ?
Or is it something else?

Anyway, there are a couple of things in your code (apart from the use of the deprecated mysql_ extension instead of mysqli_ or pdo) that I believe should be changed:

  1. If all the code you posted is one script, then don’t close and open the same database connection multiple times. Just open it at the beginning, once the script is finished it will be closed automatically
  2. Don’t put the ‘or die()’ construction on the mysql_num_rows, but on the mysql_query
  3. From the looks of it, I’d say you can join the two queries from the Country Check in one (join)
  4. As far as I can see, the while loop with $i and $num is completely useless, all you need is the while($row = mysql_fetch_array($result)) to loop through the query result set

The code would become


<?php

require("./databaseconnection.php"); //Get Database Login Information

//Do IP Country Check
mysql_connect(localhost,$username,$password);
@mysql_select_db($database) or die( "Oops theres an error, our highly trained monkeys have been notified.");
$query = "
  SELECT 
      `leads`.* 
    , `programs`.`Country`
  FROM `leads`
  INNER JOIN `programs` 
  ON `programs`.`ID` = `leads`.`ProgramID`
  WHERE `leads`.`CountryCheck` = 'NO'
";		
$result=mysql_query($query) or die(mysql_error() . ' in query ' . $query);
while ($row = mysql_fetch_array($result)) {
  $ipcountry=$row['IPCountry'];
  $ipcc=$row['IPCC'];
  $ipisp=$row['IPISP'];
  $leadid=$row['LeadID'];
  $ProgramID=$row['ProgramID'];
  $programcountry=$row['Country'];
  if ($ipcc == $programcountry) {
    $query3 = "UPDATE leads SET CountryCheck='PASS' WHERE LeadID='$leadid'";
  } else {
    $query3 = "UPDATE leads SET CountryCheck='FAIL' WHERE LeadID='$leadid'";
  }
  mysql_query($query3) or die(mysql_error() . ' in query ' . $query);
}		 
echo "Finished Country Check! <br />";

//Do Email Valid Check
include("./emailcheck.php");
$query = "SELECT * FROM `leads` WHERE `EmailCheck` = 'NO'";		
$result=mysql_query($query) or die(mysql_error() . ' in query ' . $query);
while ($row = mysql_fetch_array($result)) {
  $emailaddress=$row['EmailAddress'];
  $leadid=$row['LeadID'];
	  
  $bIsEmailValid = jValidateEmailUsingSMTP("$emailaddress", "REMOVED.com", "fraudcheck@REMOVED.com");

  $emailchecker = $bIsEmailValid ? "Valid" : "Invalid";
		
  if ($emailchecker=="Valid") {
    $query3 = "UPDATE leads SET EmailCheck='PASS' WHERE LeadID='$leadid'";
  } else {
    $query3 = "UPDATE leads SET EmailCheck='FAIL' WHERE LeadID='$leadid'";
  }
  mysql_query($query3) or die(mysql_error() . ' in query ' . $query);
}		 
echo "Finished Email Check!";

?&gt;
3

Hi,

Thanks for your response and help so far, what i was saying was if there was no rows for the Country Check then for some reason then Email Check script would not run when it should still run. But i guess thats because of this line:

$num=mysql_numrows($result) or die(mysql_error());

??

Also i’ve ran your MySQL Query you suggested to use but I’m not quiet sure what was wrong with the one i was already using? Can you let me know why i should use your MySQL Query and what the difference is or any advantages of using it please.

Thank you.

4 
while ($row = mysql_fetch_array($result)) {
  $emailaddress=$row['EmailAddress'];
  $leadid=$row['LeadID'];
      
  $bIsEmailValid = jValidateEmailUsingSMTP("$emailaddress", "REMOVED.com", "fraudcheck@REMOVED.com");

  $emailchecker = $bIsEmailValid ? "Valid" : "Invalid";
        
  if ($emailchecker=="Valid") {
    $query3 = "UPDATE leads SET EmailCheck='PASS' WHERE LeadID='$leadid'";
  } else {
    $query3 = "UPDATE leads SET EmailCheck='FAIL' WHERE LeadID='$leadid'";
  }
  mysql_query($query3) or die(mysql_error() . ' in query ' . $query);
}

This is just a personal preference, but Instead of all that just have something like:


foreach (mysql_fetch_array($result) as $row) {      
  $flag = (jValidateEmailUsingSMTP($row['EmailAddress'], "REMOVED.com", "fraudcheck@REMOVED.com")) : "PASS" : "FAIL";
  $update_flag = "UPDATE leads 
                        SET EmailCheck='$flag' 
                        WHERE LeadID='  . $row['EmailAddress'] . '";
  mysql_query($update_flag) or die(mysql_error() . ' in query ' . $query);
}

Here are some of the reasons I say that:

You are only using these vars once, what is the point of re-assigning them to another var?


  $emailaddress=$row['EmailAddress'];
  $leadid=$row['LeadID'];

jValidateEmailUsingSMTP() seemingly returns only true or false, so just assess its return value, might as well subject that output to a ternary operator …

Why make the code so hard to follow re-assigning the boolean value to 2 other variables?


  $bIsEmailValid = jValidateEmailUsingSMTP("$emailaddress", "REMOVED.com", "fraudcheck@REMOVED.com");

  $emailchecker = $bIsEmailValid ? "Valid" : "Invalid";
        
  if ($emailchecker=="Valid") {
// etc
}

There should be no need to quote a string argument which is a string …

jValidateEmailUsingSMTP("$emailaddress", …

This is a meaningless variable name, give it a name that means something, maybe you can do better than $update_flag?


$query3 =

Those 2 lines are exactly the same except for the EmailCheck value. Identify that which changes. Eradicate duplicate code when you can.


    $query3 = "UPDATE leads SET EmailCheck='PASS' WHERE LeadID='$leadid'";
  } else {
    $query3 = "UPDATE leads SET EmailCheck='FAIL' WHERE LeadID='$leadid'";
  }

Lastly format your SQL statements using multiple lines like that and you will find that when you come back later to scan your code looking to solve a problematic SQL error, the SQL statements jump off the page at you - as opposed to just melding in with the PHP and other code.

These are arguably minor insignificant points, but taken together I find these practices make my code easier to read.

You and others may well not agree, and you are of course free to disagree or totally ignore me – but having re-written so much of my own code since reading Bob Martins “Clean Code”, I can’t help but say something when I see so many instances in one place.

Maybe this helps you or not – but hopefully it makes you think about getting the balance right between readability and springing to life unnecessary variables which just make your code harder to read and follow.

5

Running one query is much faster than running lots of queries. If you have lots of leads to check, then running a query inside the loop could really slow things down.

As far as the email check not running, I have no idea. Does the same thing happen with my code?
Do you see the “Finished Country Check!” and “Finished Email Check!” messages appear?