1

I'm quite new to php and mysql so hopefully someone with more experience will be able to give me some guidance here.

I have the following code:

<?php

$npcname = $_GET['npcname'];
$npcinfo="SELECT * from npcs where name='$npcname'";
$npcinfo2=mysql_query($npcinfo) or die("could not get npc!");
$npcinfo3=mysql_fetch_array($npcinfo2);

$listquests = "SELECT * from quests where npcid = '$npcinfo3[npcid]'";
$listquests2 = mysql_query($listquests) or die("No Quests to list");
$listquests3=mysql_fetch_array($listquests2);

echo "<b>Quests Available for ".$npcname."</b><br>";

while($row=mysql_fetch_array($listquests2)) {

echo $row['name'];


}
?>

To go with this I have some tables whcih look like this:

npcs
name|location|npcid

quests
name|qid|npcid

So a quest is associated to a NPC via the npcid field.

I have one entry in each table.

Bob|Scrapyard|1
AND
Sort Scrap Metal|1|1

As you can see the quest and Bob both share the npcid of 1.

In my loop I am trying to list all of the quests for Bob. However on running the code I do not get any quests listed.

If I put the code:

  $listquests3['name'];

Outside of my loop it successfully displays "Sort Scrap Metal" as expected. The reason I have used the loop is to display multiple quests when I add them.

If somebody could be kind enough to take a look at the code and tell me what I have done wrong I would be grateful.

Thank You.

Haim Evgi
  • 123,187
  • 45
  • 217
  • 223
GesterX
  • 13
  • 3
  • +1 for making something involving quests and npc's for (one of) your first project(s). – Spiny Norman Dec 14 '10 at 12:44
  • The sample code is vulnerable to [SQL injection](http://unixwiz.net/techtips/sql-injection.html), which is a very serious [security risk](http://bobby-tables.com/). To fix this hole, switch from the outdated mysql driver to [PDO](http://php.net/PDO) and use [prepared statements](http://www.php.net/PDO.prepared-statements). If you need a PDO tutorial, try ["Writing MySQL Scripts with PHP and PDO"](http://www.kitebird.com/articles/php-pdo.html). The site you save may just be your own. As for [`or die`, don't use it when outputting HTMl](http://www.phpfreaks.com/blog/or-die-must-die). – outis Dec 14 '10 at 13:40
  • Make sure you read about [joins](http://en.wikipedia.org/wiki/Join_%28SQL%29), as others have mentioned. Issuing multiple statements when one will do is inefficient and wastes resources. Let the database handle data operations and the program handle everything else. Lastly, don't use [`SELECT *`](http://stackoverflow.com/questions/321299/what-is-the-reason-not-to-use-select); select only the columns you need. – outis Dec 14 '10 at 13:48
  • Thank You for the tips. I will indeed read the PDO articles and switch to joins for efficiency. – GesterX Dec 14 '10 at 14:43

5 Answers5

2

It may be a good idea to print out the SQL and run this against your database to see what results you get. Looking at this it looks like there may only be one result which is fetched in the

$listquests3=mysql_fetch_array($listquests2);

line. Since there are no more results there is nothing to loop over.

Jim
  • 22,354
  • 6
  • 52
  • 80
  • Ah thank you very much! I didn't realise that when I fetched the intial result that I would be excluding it from further fetches. – GesterX Dec 14 '10 at 12:55
0

The results are already fetched, since there is only one rule and you fetched it in $listquests3 :). It will work if you remove that line I think.

Lucas Moeskops
  • 5,445
  • 3
  • 28
  • 42
0

You need to do a INNER JOIN or a LEFT JOIN. Yes after carefully seeing the question again, I found that when doing the "mysql_fetch_array()" code for the first time (just before the "while" loop), the value of the variable "$listquests2" gets lost. So the "while" loop does nothing fruitful.

You must remove this single line for variable "$listquests3".

Knowledge Craving
  • 7,955
  • 13
  • 49
  • 92
0

You only have one row, and you fetched that row when you called mysql_fetch_array the first time. When you call it the second time, there are no more rows to fetch in the result set, the function returns false and your loop exits.

Dan Grossman
  • 51,866
  • 10
  • 112
  • 101
0

This statement: "$listquests3=mysql_fetch_array($listquests2);" already fetches the first. Sicne you have only one, there's nothing more to fetch, so the next call to mysql_fetch_array will return nothing.

That should fix it, but for your own 'experience', this might be a good moment to start learning about MySQL joins (LEFT JOIN in particular). You can easily find a lot about it on the internet!

Jochem
  • 2,995
  • 16
  • 18