My php loop is looping 11 times, instead of one. Cant find the bug

Question

Here is the php code. The functions require, query and render are given to us.

<?php

// configuration
require("../includes/config.php"); 

$rows = CS50::query("SELECT `symbol`, `shares`, `cash` FROM `portfolios`, `users` WHERE ?", $_SESSION["id"]);

$positions = [];
foreach ($rows as $row)
{
    $stock = lookup($row["symbol"]);
    $total = ($stock["price"] * $row["shares"]);

    if ($stock !== false)
    {
        $positions[] = [
            "name" => $stock["name"],
            "price" => $stock["price"],
            "shares" => $row["shares"],
            "symbol" => $row["symbol"],
            "total" => $total,
            "cash" => $row["cash"]
        ];
    }
}

// render portfolio
render("portfolio.php", ["positions" => $positions, "title" => "Portfolio"]);

Here is my HTML output

<div id="middle">
<table class="table table-striped">

<thead>
    <tr>
        <th >Symbol</th>
        <th >Name</th>
        <th >Shares</th>
        <th >Price</th>
        <th >TOTAL</th>
    </tr>
</thead>
<tbody>

    <?php foreach ($positions as $position): ?>
        <tr>
            <td align="left" ><?= $position["symbol"] ?></td>
            <td align="left" ><?= $position["name"] ?></td>
            <td align="left" ><?= $position["shares"] ?></td>
            <td align="left" ><?= number_format($position["price"], 2) ?></td>
            <td align="left" ><?= number_format($position["total"], 2) ?></td>
        </tr>
    <?php endforeach ?>

<tr>
    <td colspan="4" align="left">CASH</td>
    <td align="left"><?= number_format($position["cash"], 2) ?></td>
</tr>

</tbody>

</table>

My guess is that there is something wrong with my foreach loop. But im not quite sure, there could also be some faults in my SQL database.

My mySQL database consist of 3 rows user_id, symbol, shares. And for my user_id I have 3 diffrent stocks with like 10 shares each.

Anyone know what could be wrong?

This looks generally correct, but before blaming the foreach loop, you should you confirm that `$positions` actually contains what you think it. Try using `var_dump($positions)` before the loop, or output each row you get back from the database before/after adding to `$positions`. — jbafford, Jan 16 '16 at 14:31
Which foreach loop are you talking about? Pls add more details around the error because we cannot see your data, such as query outputs. — Shadow, Jan 16 '16 at 14:31
This is an SQL syntax error here `WHERE ?` you can't bind columns. Checking for errors would have informed you of that. And unsure if you did start the session. Error reporting will inform you of that. — Funk Forty Niner, Jan 16 '16 at 14:37
@jbafford Yes I did `var_dump` on `$positions` And it seems like my html output is correct it's `$positions` that is wrong. Do you know why? I get an array with 11 repetitions of each share, instead of just one. — mrfr, Jan 16 '16 at 14:41
@Fred-ii- I dont get any errors though, I did change it to `WHERE user_id = ?` though. Still same error — mrfr, Jan 16 '16 at 14:42
probably because you're not checking for them. I can assure you though, you can't bind columns and it's probably not making it there also. Unsure which API you're using to connect with: `mysql_`? `mysqli_`? PDO? other? — Funk Forty Niner, Jan 16 '16 at 14:45
What @Fred-ii- spotted is not a syntax error, but that causes the issue. If the id in session is a positive number, then you will have a `...where 123` which evaluates to true. This means that your where does not filter your table. — Shadow, Jan 16 '16 at 14:45
@Shadow In other words, you're saying that `WHERE ?` is valid syntax. Here, have a few reads http://stackoverflow.com/questions/15182910/php-pdo-bind-table-name --- http://stackoverflow.com/questions/11312737/can-i-parameterize-the-table-name-in-a-prepared-statement — Funk Forty Niner, Jan 16 '16 at 14:48
@Shadow `Select * from table1 where 123` and `Select * from table1 where ?` are two different animals. — Funk Forty Niner, Jan 16 '16 at 14:49
@Fred-ii- the OP is not binding table or column name here, but a number, that can be interpreted in a where criteria. — Shadow, Jan 16 '16 at 14:51
Plus the query may not even be assembled with binding. We do not know how it is prepared. If the query would raise a mysql error, then $position array would be empty. — Shadow, Jan 16 '16 at 14:53

score 1 · Answer 1 · answered Jan 16 '16 at 15:49

If it runs 7 times it means there are 7 rows returned, the foreach loop is fine. Most likely the issue is here:

 $rows = CS50::query("SELECT `symbol`, `shares`, `cash` FROM `portfolios`, `users` WHERE ?", $_SESSION["id"]);

You aren't indicating which column should equal the id, so it's returning all of them likely. Need something like this:

$rows = CS50::query("SELECT `symbol`, `shares`, `cash` FROM `portfolios`, `users` WHERE portfolios.user_id = ?", $_SESSION["id"]);

My php loop is looping 11 times, instead of one. Cant find the bug

1 Answers1