Moving foreach loop inside the while loop is not working in the expected way

Question

I need some help and clarification about how to do it all correctly, since it look like that I am doing it in the wrong and not most optimal way so it wasted my server resource.

Here is my code, which works, but I think it could be optimized for much faster performance if foreach is placed inside the while loop, instead of wrapping it like this :

$link_id = explode(',', $ref_links_id);
    foreach($link_id as $key => $value){
        $additional_links_query = 'SELECT * FROM additional_links WHERE id = '.$value.' ORDER BY ID asc';
        $res = $db->prepare($additional_links_query);
        $res->execute();
        while ($info = $res -> fetch()){
            $li_text = ($info['_ad_text'] == NULL)? '' : ' - '.$info['_ad_text'];
            $li_target = ($info['_ad_url_target'] == NULL)? '' : ' target="'.$info['_ad_url_target'].'"' ;
            $li_nofollow = ($info['_ad_nofollow'] == 1)? ' rel="nofollow"' : '';
            $li_cont = '<li><a href="'.$info['_ad_url'].'"'.$li_target.$li_nofollow.'>'.$info['_ad_anchor'].'</a>'.$li_text.'</li>';
        print<<<END
        $li_cont\n
        END;
    }
}

My initial approach was to place foreach loop inside the while loop, which could have save the round backs, but every attempt of mine has failed.

mrash · Accepted Answer · 2014-04-27T13:43:49.107

1

If I understand correctly, you need to remove one of the foreach or while loop from your code

see this code that I remove foreach from PHP and add IN command to your SQL query:

$additional_links_query = 'SELECT * FROM additional_links WHERE id IN ('.implode(",", $ref_links_id).') ORDER BY ID asc';
$res = $db->prepare($additional_links_query);
$res->execute();
while ($info = $res -> fetch()){
    $li_text = ($info['_ad_text'] == NULL)? '' : ' - '.$info['_ad_text'];
    $li_target = ($info['_ad_url_target'] == NULL)? '' : ' target="'.$info['_ad_url_target'].'"' ;
    $li_nofollow = ($info['_ad_nofollow'] == 1)? ' rel="nofollow"' : '';
    $li_cont = '<li><a href="'.$info['_ad_url'].'"'.$li_target.$li_nofollow.'>'.$info['_ad_anchor'].'</a>'.$li_text.'</li>';
print<<<END
$li_cont\n
END;

edited Apr 27 '14 at 13:43

answered Apr 27 '14 at 13:34

mrash

883
7
18

problem is that $ref_links_id is an array and it wont work like that. What I actually needed is to move foreach inside the while loop instead of having while inside the forach loop. – AlexB Apr 27 '14 at 13:39
okey, I change `$ref_links_id` to `implode(",", $ref_links_id)` – mrash Apr 27 '14 at 13:43
`IN()` requires `()` around the value list. Also note if the values are strings, this will fail, and `'A,B,C'` will need `FIND_IN_SET(id, 'A,B,C')` instead. – Jared Farrish Apr 27 '14 at 13:44
@AlexB won't be notified of your question unless you notify with `@AlexB` in your comment. – Jared Farrish Apr 27 '14 at 14:46

score 1 · Answer 2 · edited May 23 '17 at 12:20

It sure looks like you could do this in a single query, as well as using MySQLI correctly using Prepared Statements:

$ref_links_arr = explode(',', $ref_links_id);

// We need a series of parameter placeholders, e.g., ?,?,?
// One for each ref_link_id, which will be used in the
// query as placeholders in the IN() clause list.
$ref_links_params = implode(',', array_fill(0, count($ref_links_arr ), '?'));

// Using IN(), see the links below for the MySQL manual entry.
$additional_links_query = "
SELECT * 
FROM additional_links 
WHERE id IN($ref_links_params) 
ORDER BY ID asc
";

$res = $db->prepare($additional_links_query);
$res->execute();

// Here, we're going to replace each ? placeholder with
// a corresponding ref_link_id from $ref_links_arr.
while ($ref_link_id = array_shift($ref_links_arr)) {
    // "i" is for "integer", so if your id field is
    // string, use "s"
    $res->bind_param("i", $ref_links_id);
}

while ($info = $res->fetch()){
    $li_text = ($info['_ad_text'] == NULL)? '' : ' - '.$info['_ad_text'];
    $li_target = ($info['_ad_url_target'] == NULL)? '' : ' target="'.$info['_ad_url_target'].'"';
    $li_nofollow = ($info['_ad_nofollow'] == 1)? ' rel="nofollow"' : '';
    $li_cont = '<li><a href="'.$info['_ad_url'].'"'.$li_target.$li_nofollow.'>'.$info['_ad_anchor'].'</a>'.$li_text.'</li>';

    print "$li_cont\n";
}

score 0 · Answer 3 · edited May 23 '17 at 12:11

0

comment line

$res->execute();

because you are try to execute and fetch also.. PHP's PDO execute() vs. fetch()?

edited May 23 '17 at 12:11

Community

1
1

answered Apr 27 '14 at 13:14

poh

171
9

score 0 · Answer 4 · answered Apr 27 '14 at 13:19

You're already using a prepared statement, you might as well bind your data before injecting to your database:

$additional_links_query = 'SELECT * FROM additional_links WHERE id = :value ORDER BY ID asc';
$res->bindValue(':value',$amount);
$res = $db->prepare($additional_links_query);
$res->execute();

If you don't want to loop through your results straight away, you could do PDO::FETCH_ASSOC. Of course, this will return an associative array which you will have to parse at a later time.

$res->fetch(PDO::FETCH_ASSOC);
print_r($res);

Moving foreach loop inside the while loop is not working in the expected way

4 Answers4