Call a PHP function inside of a while loop

Question

I have a function which gets a particular set of users from a table where a particular WHERE condition is meet.

I need to send each of them a message.

So, I used another function to send the message. And called that function inside he following while loop

while($user= mysqli_fetch_assoc($users_set)){
   send_message($user['email']);
}

So, the problem is, the function is called only just one time. (Only with the last value of the loop)

How to fix this problem and make the function called with each value of the loop...

This is the full code...

$query = "SELECT * ";
$query .= "FROM user ";
$query .= "WHERE confirmed = 0";

$user_set = mysqli_query($db_conx, $query);
confirm_query($user_set);

while($user = mysqli_fetch_assoc($user_set)){
   send_message($user['email']);
}

Here is the send message function....

function send_message($email){
global $db_conx;

$invitee_user = get_user_by_email($email);

$query5 = "INSERT INTO notification(";
$query5 .= "description, user_id";
$query5 .= ") VALUES(";
$query5 .= "'You have been confirmed'";
$query5 .= ", {$invitee_user['id']}";
$query5 .= ")";

$result5 = mysqli_query($db_conx, $query5);

if($result5){
    //$_SESSION["message"] = "Notification sent". \mysqli_error($db_conx);
    return "OK";
}else{
    //$_SESSION["message"] = "Failed to send notification". mysqli_error($db_conx);
}

}

Here is the code for confirm_query()

function confirm_query($result_set){
if(!$result_set){
   die("Fatal Error Occured : Database Query Failed <a href=\"error-report.php\">Report this error</a>"); 
}

}

On this one iteration, does $user have a valid value? According to the PHP docs, the function mysqli_fetch_assoc() does not even exist: php.net/… — TimoStaudinger
– TimoStaudinger, Commented May 14, 2015 at 19:46
You should post code for confirm_query() and send_message(); It could be that something in these functions in either impacting the DB connection or causing execution to abort. — Mike Brant
– Mike Brant, Commented May 14, 2015 at 19:54
Not a good idea to do expensive operation when locking the database up. Grab the rows and then send the email — Ed Heal
– Ed Heal, Commented May 14, 2015 at 20:00

Mike Brant · Accepted Answer · 2015-09-30 15:56:37Z

5

I would just boil this down to one query and get rid of all the looping stuff

INSERT INTO notification (description, user_id)
SELECT 'You have been confirmed', user_id
FROM user
WHERE confirmed = 0

Your current logic is really convoluted.

You query the user table to get the user email field, then pass that email as parameter to your function only to then turn around and (I presume) look up the user ID based on email (when you already had this information from your initial query), then you make insert.

This means that for every record you return from first query, you need to do 2 queries to insert to the notification table. So if you had 100 results you would end up doing a total of at least 201 queries to complete the insertions.

Using my approach you make 1 query regardless of how many rows are affected.

One takeaway that you should get from this is that, anytime you see yourself trying to do some sort of nested querying, you should recognize this as an anti-pattern (a coding pattern that you do not want to typically use). There is usually a better approach that can be taken if you rethink how you are writing your queries.

edited Sep 30, 2015 at 15:56

answered May 14, 2015 at 20:05

Mike Brant

71.4k10 gold badges102 silver badges104 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Tharindu Thisarasinghe Over a year ago

Wow.. Haven't thought of this much of tricky sql command.... Thanks.. I'll give it a try... :)

Mike Brant Over a year ago

@TharinduLucky No problem. I did add update to my answer that you should look over. The real takeaway for you is learning that this is an anti-pattern you should recognize and rethink your approach when you encounter it.

Tharindu Thisarasinghe Over a year ago

I tried your answer, sure it opened my eyes. But, I ended up having another problem. Here I want to insert to notification table more than one row. How many users found with confirm = 0, that will be the number of records to be added to the notification table. Because I need to include some details specific to user in the description field of notification table. So, where to go now ?

Mike Brant Over a year ago

@TharinduLucky It depends on what that user-specific information is and what it is generated from (does it already exist in user table?). Your example just showed a static string (You have been confirmed) being inserted, so my answer was based on that.

Tharindu Thisarasinghe Over a year ago

Yeah, when I was asking the question I was just focusing on calling the function in the loop. Sorry about that. Anyway, I managed to do what I wanted with a foreach loop. First I get user specific emails (according to the condition) and store them in a array. Then loop through that array using a foreach and call the function inside it. However, this may not be the smartest way of doing this. What do you think anyway ?

|

Collectives™ on Stack Overflow

Call a PHP function inside of a while loop

1 Answer 1

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related