How to solve PHPError: Undefined variable on line 40?

Question

These are the first 41 lines of my code and the error I keep getting just says:

[28-Oct-2015 18:41:39 UTC] PHP Notice: Undefined variable: item in C:\MAMP\htdocs\GameDevProject\Inventory\dbFunctions.php on line 41

I can't seem to figure out what I need to fix it

function open_database_connection() {
// DB connection details
$username = 'user';
$password = 'pass';
$host = 'localhost';
$db = 'inventory';

// try to connect to DB
$connection = mysqli_connect($host, $username, $password, $db);

// check connection successful
// redirect with message to error page
if (mysqli_connect_errno()) {
$errorMessage = 'DB connection failed: '.mysqli_connect_error();
die($errorMessage);
}
return $connection;
}

function close_database_connection($connection)
{
mysqli_close($connection);
}

function getitemByPlayerName($playerName){
$connection=open_database_connection();
// $item = getitemByPlayerName($playerName);
$query = "select * from inventory WHERE player='$playerName'";
// die($query);
if($result = mysqli_query($connection,$query)){
if($row = mysqli_fetch_assoc($result)){
$item = $row['item'];
}
} else {
$errorMessage = 'Query failed:'.$query;
die($errorMessage);
}
close_database_connection($connection);
return $item;
}

Answer 1

I agree with what Ben wrote.



Just look at your code line by line.



First line:

if($result = mysqli_query($connection,$query)){



Execute the query. If it's successful, then continue.



Next line:

if($row = mysqli_fetch_assoc($result)) {



If mysqli_fetch_assoc() returns a result, then continue. Read that first sentence again. If mysqli_fetch_assoc RETURNS A RESULT, then continue. This is your error--there are no rows being returned from your query. If you were to modify this line to this:



if($row = mysqli_fetch_assoc($result)){

$item = $row['item'];

} elese {

echo 'Player not found!';

}



You would see that no rows are being returned. That means you didn't type a username that exists.



Also, and this is much more important than your problem at hand, you really need to look into using some object-oriented PHP, especially PHP's built-in MySQLi class. This will save you a lot of headaches and clean up your code a lot. As it stands, for every function you create, you'll have to open and close a database connection for EVERY query. This is incredibly inefficient and will slow your server to a crawl if you had a few users using it. I've linked to a resource on how to use PHP's MySQLi class.

Answer 2

My guess is that when you did the query, it either didn't evaluate $playerName into a string, leaving it as literally $playerName, or what ever player name is being looked for doesn't exist. Either way, when you never end up running "$item = $row['item'];", and as such, when you reach the end of the script, it tries to return a variable, $item, which was never defined.



I'd add some debug print functions, or write to a log you can check, that print when you enter if blocks, or the current values of variables. I've done that to see the exact moment code behaved in a way I wasn't expecting, which then tells me what part of the code isn't working.



Edit: Ok, reformatted the code to add indentation (Thank you Yahoo! for taking that away ...) so it's easier to read. Looks like if it fails the search, it'll exit with an error message, so that seems unlikely. Beyond that, I'm not too familiar with PHP, and I'm not seeing any obvious reasons for why $item would be undefined. Once again, this is where debugging can be very helpful. Add multiple statements that show the current state of the program at various lines. A print statement just before the line "$item = $row['item']" would let you know if that part got executed. Trying to print $row['item'] directly would let you know what you're trying to store to $item, and if anything is being stored at all.

Answer 3

Several problems

1, and the more obvious:

function getitemByPlayerName($playerName){

$connection=open_database_connection();

// $item = getitemByPlayerName($playerName);

=> you commented it out, so $item will NEVER be set, hence the reporting of your error. (more error further down).

2. The same line (if used) actually creates a RECURSIVE call: the fuction getitemByPlayerName calls itself!

3. Against Jeff P advice: using a class to call a database is totally superfluous and, very often, lead the programmer to enter parameters to a call in incorrect form, leading the class to mess upt the lot. You don't need to open and close your DB all the time either: you make $Connection a global, and call the open/close function nly ONCE (Keep the DB open while you run the script). (I know that Jeff P is going to jump over the wall, but I maintain my position!)

=>https://www.youtube.com/watch?v=o9pEzgHorH0

4. Try this:


$Conn; // define a global.

function open_database_connection()

{

global $Conn;

$username = 'user';

$password = 'pass';

$host = 'localhost';

$db = 'inventory';

$Conn = mysqli_connect($host, $username, $password, $db) or die("Error " . mysqli_error($Conn));

// => This will open the DB, store the link in $Conn, and stops execution if fail to connect. No need to return $Conn, as it is a global.

}

function close_database_connection()

{

global $Conn;

mysqli_close($Conn);

}

function getitemByPlayerName($playerName)

{

global $Conn;

open_database_connection(); // only if it has not been open before!

$query = "select * from `inventory` WHERE `player`='" . $playerName . "'";

// => Note the syntax!

if ($res = mysqli_query($Conn, $query))

{

$row = mysqli_fetch_array($res);

$item = $row['item'];

}

else

{

$errorMessage = 'Query failed:'.$query;

die($errorMessage);

}

close_database_connection(); // => only if you don't need it anymore.

return ($item);

}

Answer 4

STOP!

Learn to use stored procedures or at the minimum binding SQL parameters before continuing. Your code has a gaping hole that allows for a SQL injection attack.

Guess what happens if an attacker turns $playerName from joe to joe' or 'a'='a