PHP - Spot the problem in this foreach?

I am creating a simple order cart.

In essence, a user clicks order, the products id is added to a session variable ($order) as an array. When viewing the "cart" a check occurs where if a value in the session variable is equal to the id of a specific row in a mysql table, that specific row is returned as an item that was ordered. For each value in the session variable, this process should reoccur.

Here is the bit of开发者_开发知识库 code that I am battling with:

foreach ($order as $item) 
{
    while($row = mysql_fetch_assoc($productsSql))
    {
         $itId = $row['id'];
         $itDesc = $row['desc'];
         $itPrice1 = $row['price1'];
         if ($item == $itId) 
        {
        $pageContent .= '
                <tr>
                    <td>'.$itDesc.'</td>
                    <td>
                        R'.number_format($itPrice1, 2).'
                    </td>
                </tr>
';      
        }
    }   
}

The "$orders" variable is this:

session_start();
if (!isset($_SESSION['order']))
{
$_SESSION['order'] = array();
}

Can anyone spot any problems with this?

$productsQuery = 'SELECT `id`, `refCode`, `desc`, `pack`, `measure`, `quantity`, `deptCode`, `taxable`, `price1`, `price2`, `crdCode`, `cost1`, `cost2` FROM `products` ORDER BY `desc` ';
$productsSql = mysql_query($productsQuery) or die(mysql_error());`

while($row = mysql_fetch_assoc($productsSql))

This will not be reproduced every $item, so save the rows in an array before foreach and replace this while with foreach.

try this

mysql_data_seek( $productsSql, 0);

foreach ($order as $item) 
{
mysql_data_seek( $productsSql, 0);  //<- this line, to reset the pointer for every EACH.
    while($row = mysql_fetch_assoc($productsSql))
    {
         $itId = $row['id'];
         $itDesc = $row['desc'];
         $itPrice1 = $row['price1'];
         if ($item == $itId) 
        {
        $pageContent .= '
                <tr>
                    <td>'.$itDesc.'</td>
                    <td>
                        R'.number_format($itPrice1, 2).'
                    </td>
                </tr>
';      
        }
    }   
}

hope that helps

You haven't specified the contents of $productsSql, but the line containing it looks wrong.

If $productsSql contains SQL code then it won't work at all, because mysql_fetch_assoc() requires a database resource. To get a resource, you need to call mysql_query() with your SQL code.

Assuming you have called mysql_query() outside of the code you've quoted, and $productsSql does contain the resulting resource, it's still wrong, because you're looping through the database results multiple times (because of the foreach loop), but without resetting or requerying.

The upshot of this is that after the first iteration of the foreach, the while loop will have run through all the results in the database query, and will be set to the end of the result set. Looping back to the top of the code won't change the position of the result set, so the first time it tries to call mysql_fetch_assoc() in the second loop through, it will say "I've already finished", and nothing will happen.

If you want the same set of results to be processed with each iteration of the foreach loop, then you need to reset the DB pointer before starting the while loop. This can be done with the mysql_data_seek() function.

This will probably achieve what you want, with the code you've provided.

However, this may not be the most efficient way of achieving your desired result. Unfortunately, it's hard to be any more specific how to improve it without seeing more of your code (in particular, the SQL query and more of your data structure).

[EDIT] After the OP added the SQL query to the question, we can see more clearly what the best course of action is.

A quick and dirty fix would indeed be to use mysql_data_seek() as I explained above (and as shown in code by @Phoenix), but while this will make the program work, it will be very inefficient, especially as more products are added to the database.

What is actually required is a complete change around of the code, starting with the SQL query itself.

The query needs to be changed to only include the items that are actually wanted. This would be done using a WHERE clause, so the query could look something like this:

SELECT <lots of fields> FROM products WHERE id IN(5,7,9,<etc>)

This is easy to produce, since $orders appears to be an array of item IDs, so you can simply use implode(',',$orders) to make a list of IDs suitable for the WHERE clause. (you would need to be absolutely certain that they were all numeric IDs, though, because anything else would break the SQL query; if you're not sure, you should SQL escape them first).

Once you've got that list, the best approach is to load the DB values into a memory array, and then use your foreach($order) loop to grab the appropriate array element.

So your final code would look something like this:

$productsQuery = 'SELECT `id`, `refCode`, `desc`, `pack`, `measure`, `quantity`, `deptCode`, `taxable`, `price1`, `price2`, `crdCode`, `cost1`, `cost2` FROM `products` WHERE id IN('.implode(',',$orders).')';
$productsSql = mysql_query($productsQuery) or die(mysql_error());`

$data = array();
while($row = mysql_fetch_assoc($productsSql)) {
    $data[$row['id']] = $row;
}

foreach($order as $item) {
   $pageContent .= '
            <tr>
                <td>'.$row[$item]['desc'].'</td>
                <td>
                    R'.number_format($row[$item]['price'], 2).'
                </td>
            </tr>
    ';
}