5

I'm just experimenting a little with PHP and PDO working with a MySQL database and I'm a little stumped as to why after getting the results, storing them correctly in a multi-dimensional array and looping through them it outputs one of the array data twice.

Essentially here's the query to grab the data:

SELECT b.Price, b.ImgURL, m.Name, f.ID, f.Family, f.URL FROM Products AS b INNER JOIN Manufacturers AS m ON m.ID = b.Manufacturer INNER JOIN FamilyLookUp AS l ON l.Product = b.ID INNER JOIN Families AS f ON f.ID = l.Family GROUP BY f.ID ORDER BY b.Price ASC

I was hoping with this to get 1 row returned for each Family, which it works correctly both in the PHPMyAdmin query and also when print_r() the results.

I then store in:

$families[] = array('ID' => $f['ID'], 'Manufacturer' => $f['Name'], 'Family' => $f['Family'], 'URL' => $f['URL'], 'IMG' => $f['ImgURL'], 'Price' => $f['Price'], 'ScentCount' => 0);

Which also works correctly when doing a print_r() and when just looping through with a foreach loop echoing out the ID for each entry it returns 1234567 (all 7 Family IDs)

Then I run another query:

try{
$sqlCmd = "SELECT COUNT(*) FROM FamilyLookUp WHERE Family=:fID";
$s = $pdo->prepare($sqlCmd);
foreach($families as &$fam){
$s->bindValue(':fID', $fam['ID']);
$s->execute();
$fam['ScentCount'] = $s->fetchColumn();
}
}

This also gets the correct counts and properly stores them in the array for the number of items within each family. So all good up to now.

The problem occurs when I:

foreach($families as $fam):
        ?>

        <div class="product-listing">
        <?php echo $fam['ID']; ?>
            <div class="product-listing-image">
                <a href="<?php echo $fam['URL']; ?>"><img alt="" src="<?php echo $fam['IMG']; ?>"></a>
            </div>
            <div class="product-listing-details">

                <a href="<?php echo $fam['URL']; ?>"><h3><?php echo strtoupper($fam['Manufacturer']); if($fam['Family'] != ""){ echo strtoupper(' - ' . $fam['Family']);} ?></h3></a>
                <?php if($fam['ScentCount'] == 1): ?>
                <span class="product-scent-count"><?php echo $fam['ScentCount']; ?> Scent</span>
                <span class="product-price-value">£<?php echo $fam['Price']/100; ?></span>
                <?php elseif($fam['ScentCount']>1): ?>
                <span class="product-scent-count"><?php echo $fam['ScentCount']; ?> Scents</span>
                <span class="product-price-value">From £<?php echo $fam['Price']/100; ?></span>
                <?php endif;?>
            </div>
        </div>

        <?php
            endforeach;
        ?>

After doing this, it outputs correctly for the first 6 families of data, but for some reason it outputs a duplicate of the 6th instead of the actual 7th. When doing a print_r of all the data in the line before the start of the foreach loop, it returns all the correct data and yet within the foreach loop there becomes 1 duplicate array in the place of the 7th originally correct array.

Any advice would be awesome.

Edit for Kohloth's answer(the print_r followed directly by foreach vardump):

Array
(
    [0] => Array
        (
            [ID] => 1
        )

    [1] => Array
        (
            [ID] => 7
        )

    [2] => Array
        (
            [ID] => 2
        )

    [3] => Array
        (
            [ID] => 3
        )

    [4] => Array
        (
            [ID] => 4
        )

    [5] => Array
        (
            [ID] => 6
        )

    [6] => Array
        (
            [ID] => 5
        )

)
            array(7) {
  ["ID"]=>
  string(1) "1"
}

    array(7) {
  ["ID"]=>
  string(1) "7"
}

    array(7) {
  ["ID"]=>
  string(1) "2"
}

    array(7) {
  ["ID"]=>
  string(1) "3"
}

    array(7) {
  ["ID"]=>
  string(1) "4"
}

    array(7) {
  ["ID"]=>
  string(1) "6"
}

    array(7) {
  ["ID"]=>
  string(1) "6"
}
wsjlisseter
  • 99
  • 1
  • 1
  • 10
  • guessing : add `unset($fam);` after the foreach loop: `foreach ($families as &$fam){ ...`. You could stop using `bindParam` also. Instead use. 'bindValue' or just pass the values in the 'execute' statement? imo, you _very rarely_ need to use 'bindParam' with PDO. imo, you don't often need 'bindValue' either. imo, Just pass the data array in the `execute`. maybe intereresting? [PDO made really easy most of the time?](http://stackoverflow.com/a/33069595/3184785) – Ryan Vincent Apr 16 '16 at 19:18
  • Thanks for your comment Ryan. I am using bindValue already. Relatively new to learning PHP though so will definitely look more at just passing the data array in the execute method. Kind of interesting that when I add unset($fam) at the end of the foreach loop kohloth suggested it changes the output. From outputting the arrays in order of 1723466 = duplicate family with ID 6, to 1723461 = duplicate family with ID 1 – wsjlisseter Apr 16 '16 at 19:43
  • Oh my goodness. Thanks so much for your help. The unset thing actually worked when placed after the end of the foreach loop, I thought you were meaning inside the loop after the end of the executable code. I will definitely try to use a foreach loop only when needed from within a function. I can't imaging having too many references kicking about is that great anyway. Many thanks – wsjlisseter Apr 16 '16 at 20:17
  • No, use foreach loops a lot! **Do not use '&$var' in PHP unless you really have to**. And you should not have to. Glad you got it sorted out - thanks for listening :) – Ryan Vincent Apr 16 '16 at 20:19
  • See the answer provided by @trincot to appreciate what is happening. :) – Ryan Vincent Apr 16 '16 at 20:42
  • oh, thanks for the clarification. :) – wsjlisseter Apr 16 '16 at 21:17

2 Answers2

13

This is what is happening:

At the last iteration of this loop:

foreach($families as &$fam){
    $s->bindValue(':fID', $fam['ID']);
    $s->execute();
    $fam['ScentCount'] = $s->fetchColumn();
}

... $fam refers to the last element of the $families array.

Then when your next loop starts:

foreach($families as $fam){

... the memory location to which $fam points does not change, it remains locked to the last element of the $families array. And so in the first iteration, the content of the first element is copied into $fam, i.e. in the last entry, then upon the second iteration, the second value is overwritten there, and so on. When the last iteration starts, the last element contains the one-but-last value, and this gets overwritten by ... itself, which is again the one-but-last value.

This bug report raises the same issue, and the answer given is that this is intended behaviour. In answer to one of the duplicate bug reports, this is stated quite to the point:

The current implementation is consistent. Granted, not very useful, but it would be inconsistent to arbitrarily break the reference here.
PHP has no block scope, and breaking the reference would introduce a special-case block-scope here.

This blog explains the same behaviour with nice illustrations.

The solution is to just use another, new variable in your second loop, like this:

foreach($families as $fam2){

Alternatively, and safer for any other code where you might use $fam, is to unset($fam) just before the second loop, like this:

unset($fam);
foreach($families as $fam){

This works because at the start of the foreach loop, the variable is recreated from scratch, and thus points to its own, new memory location.

The documentation on foreach has a warning about this behaviour and suggests to unset:

Warning

Reference of a $value and the last array element remain even after the foreach loop. It is recommended to destroy it by unset().

All this is quite awkward, and when you read the replies to the related "bug" reports, it is clear that you're not the only one bumping into this unexpected side-effect. Therefore I would like to stress this point:

Avoid the use of the dangerous &

Hardly ever is it really needed to use this & prefix. By avoiding it, these types of weird side-effects will belong to the past.

Your code could be rewritten without & as follows:

foreach($families as $i => $fam){
    $s->bindValue(':fID', $fam);
    $s->execute();
    // use the index to put the value in place in the array:
    $families[$i]['ScentCount'] = $s->fetchColumn();
}

Note that it does not harm the readability of your code either.

trincot
  • 317,000
  • 35
  • 244
  • 286
  • Thank you for the rather more clear explanation of what is happening. – Ryan Vincent Apr 16 '16 at 20:32
  • Thanks for the detailed explanation. I just assumed the $fam only existed within the loop. Pretty cool, will definitely be reading up the linked material and then some! Many thanks – wsjlisseter Apr 16 '16 at 21:27
-1

For what its worth, this second pair of eyes sees nothing wrong with the HTML output portion of the code. No part of that is changing the structure of the array you are iterating, so it seems to me that maybe your diagnosis is not right? It could be worth commenting out the whole of the last foreach loop, and replacing it with:

<?php foreach($families as $fam): ?>
    <?php var_dump($fam);?> 
<?php endforeach; ?>

Just to double check that the $families array is correct before you enter the HTML-heavy foreach block.

Edit: I have run the view code with an explicit array declaration directly before it, and it does not duplicate the last result. What happens when you put this directly before your view code?

$families = [
    [
        'ID' => 1,
        'Manufacturer' => 'man1',
        'Family' => 'fam1',
        'URL' => 'url1',
        'IMG' => 'img1',
        'Price' => 'price1',
        'ScentCount' => 1
    ],
    [
        'ID' => 2,
        'Manufacturer' => 'man2',
        'Family' => 'fam2',
        'URL' => 'url2',
        'IMG' => 'img2',
        'Price' => 'price2',
        'ScentCount' => 2
    ],
    [
        'ID' => 3,
        'Manufacturer' => 'man3',
        'Family' => 'fam3',
        'URL' => 'url3',
        'IMG' => 'img3',
        'Price' => 'price3',
        'ScentCount' => 3
    ],

];

?
kohloth
  • 742
  • 1
  • 7
  • 21
  • Thanks for your response. Well it seems really weird. I've added that vardump in the for loop as suggested and it gives the same result(duplicate last array). even though it's directly after the print_r that shows the correct data. To keep it a little shorter, I have removed a load of the lines but left IDs as an edit – wsjlisseter Apr 16 '16 at 19:35
  • What happens when you explicitly declare an array of similar structure directly above your view code? – kohloth Apr 16 '16 at 19:55
  • When I use that explicitly declared array it seems to be working correctly. Any idea what could be causing it to mess up when using the database's data stored in the array I listed? – wsjlisseter Apr 16 '16 at 20:12