eMonk Posted November 17, 2011 Share Posted November 17, 2011 I recently added a mysql query and a while loop to my expiration date script but now it doesn't work. The script is suppose to send the user an email x days before it expires. <?php include("../includes/connect.php"); $query = "SELECT expiry_date FROM model"; $result = $db->query($query); $num_results = $result->num_rows; $exp_date = strtotime($row['expiry_date']); // YYYY-MM-DD $today = strtotime("now"); $expiration_date = strtotime("+3 days", $exp_date); $i=0; while ($i < $num_results); { $row = $result->fetch_assoc(); if ($expiration_date > $today) { echo "Valid: Yes<br />"; echo "Today: $today<br />"; echo "Expire date: $expiration_date"; include('../../pear/Mail-1.2.0b1/Mail.php'); ## EMAIL CODE HERE ## echo "<p>Your email has been sent to $recipients</p>"; $i++; } else { echo "No results found."; } } ?> Right now the script just echos "No results found". The mysql table has 11 entries and one of them is about to expire so the user should be sent an email but the code above doesn't work. Does anyone see anything wrong with the code above? Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/ Share on other sites More sharing options...
phporcaffeine Posted November 17, 2011 Share Posted November 17, 2011 According to your snippet; In order to see "No results found", $expiration_date has to be LESS THAN $today - because anything else would fire the logic in the construct. So, is $expiration_date GREATER THAN $today? Are you sure you know what the values are that you are comparing? Have you dumped the variables to see what they are? Do you see "No results found" on every iteration of the loop (because the condition is evaluated with each iteration)? Also, it seems that you are only incrementing the loop limiter ($i) inside the construct, when $expiration_date is GREATER THAN $today, so when $expiration_date is not GREATER THAN $today, the loop limiter is not incremented. That could cause an endless loop, if $i never gets incremented to be GREATER THAN $num_results (which is what will stop the loop). Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1288904 Share on other sites More sharing options...
PFMaBiSmAd Posted November 17, 2011 Share Posted November 17, 2011 eMonk, did you look at your own code? After you have written some code, before you ever execute it, you need to 'play computer' and proof-read/step through your code, line by line, to make sure the logic you have written does what you want. You are accessing $row['expiry_date'] before you have even set $row. How would that ever work? However, you should never retrieve all the rows from a database table, then loop through them to find a matching condition. You would use a WHERE clause in your query to retrieve just the row(s) you are interested in. You also would never put an include statement for something like the pear mail class inside of a loop as that would redefine the class every pass through the loop. You would include the mail class one time near the start of your program. Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1288910 Share on other sites More sharing options...
phporcaffeine Posted November 17, 2011 Share Posted November 17, 2011 eMonk, did you look at your own code? After you have written some code, before you ever execute it, you need to 'play computer' and proof-read/step through your code, line by line, to make sure the logic you have written does what you want. You are accessing $row['expiry_date'] before you have even set $row. How would that ever work? However, you should never retrieve all the rows from a database table, then loop through them to find a matching condition. You would use a WHERE clause in your query to retrieve just the row(s) you are interested in. You also would never put an include statement for something like the pear mail class inside of a loop as that would redefine the class every pass through the loop. You would include the mail class one time near the start of your program. Yeah, I had hoped to get OP through some of the logic of the loop, before addressing the include in the loop. It wouldn't work anyway, unless include_once was used .... regardless, not needed either way. I figured, once I got OP to figure out how to fix the loop then he/she would 'discover' that including the same class, over and over, in a loop wouldn't work, once it started throwing errors. Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1288917 Share on other sites More sharing options...
eMonk Posted November 17, 2011 Author Share Posted November 17, 2011 Thanks for the tips gents! It seems to be working now. PFMaBiSmAd: I will be using a WHERE clause in the final code but I want to retrieve all the rows from the expiry_date column to check if the user's page is going to expire within 3 days and if yes to send an email reminder to renew. Right now the table only has 11 test entries. It shouldn't be more then 100 once it goes online but if it does reach 100's of entries, is this approach right? Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1289145 Share on other sites More sharing options...
PFMaBiSmAd Posted November 18, 2011 Share Posted November 18, 2011 Using the database engine (compiled code) to find just the matching rows, will be at least 10x faster than using php (a parsed, tokenized, interpreted language) to loop over, convert (you don't actually need to convert yyyy-mm-dd dates to timestamps to compare them), and test all the values to find which ones to operate on. Plus, because the query you posted is only retrieving the expiry_date, you likely have other query(ies) inside of your php loop that is(are) retrieving related data. By using a single query (joined with related tables as needed) that gets only the data you want, you can likely make what you are doing 20x faster (important if your site becomes popular or if your web host starts threating to suspend your account for using too many resources), You will also end up with less total code that you must write, test, and maintain (important if you are getting paid to do this or when you need to make changes to the code in the future.) Database engines are designed to efficiently find data, php is not. Php is designed to be an easy to use, general purpose, programming language. Use each for what they were designed to do and you will end up with the simplest, quickest, and most efficient end result. For example, assuming that your rows in the model table contain all the needed information (otherwise this would be a JOIN query statement), the following query will get just the rows that you want - SELECT * FROM model WHERE expiry_date BETWEEN CURDATE() AND CURDATE() + INTERVAL 3 DAY The only php code would be to loop over the result from that query and use each row. There would be no php code to form or convert dates or test each date. Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1289278 Share on other sites More sharing options...
eMonk Posted November 19, 2011 Author Share Posted November 19, 2011 Thanks PFMaBiSmAd. I was looking into retrieving date ranges in sql queries last night. I was trying to find a CURMONTH operator to narrow down the results but I guess I doesn't exist because I couldn't find anything about it. I tried your query and tried to echo the number of results but it keeps returning 0. <?php include("../includes/connect.php"); $query = "SELECT expiry_date FROM model WHERE expiry_date BETWEEN CURDATE() AND CURDATE() - INTERVAL 3 DAY"; $result = $db->query($query); $num_results = $result->num_rows; echo "$num_results"; ?> $num_results should echo "1" because there is 1 expiry_date column with "2011-11-17". Any ideas? I'm trying to only fetch the rows where the expiry_date column is 3 days before today's date. Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1289446 Share on other sites More sharing options...
eMonk Posted November 19, 2011 Author Share Posted November 19, 2011 I have it working now. Had to flip the 2 CURDATE parameters around. Quote Link to comment https://forums.phpfreaks.com/topic/251303-need-help-with-while-loop/#findComment-1289457 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.