2

I've read a LOT of things about this problem, however I still cannot fix it.

In my functions file I declare a variable with a value like so:

$px_host = "localhost";

And I have a database query function like so:

function dbQuery($database, $reqquery){
if(!$connect = mysql_connect($px_host, $px_dbuser, $px_dbpass)){
    exit("Error - cannot connect to MySQL server - " . mysql_error());
}

if(!$database = mysql_select_db($database)){
    exit("Error - cannot select database - " . mysql_error());
}

if(!$query = mysql_query($reqquery)){
    exit("Error - query error.");
}

return $query;
}

And whenever I try and run the function, I get an 'Undefined Variable' error. I've tried setting the variable to global, however it still says it's undefined. They are in the same file and the variable is defined and set before the function.

(I'm bad at explaining this so try and work round it) The file which contains dbQuery() is included in another file. In the other file, there is a function which uses dbQuery() to get certain information. Is it possible that because it's being initially run from the first file, the variable is outside the scope?

Improve this question
2
  • 2
    It is a bad idea to reconnect everytime you want to send a SQL query. The first two operations in your code should be done only once at the beginning of your script. Besides that, consider using mysqli or PDO which are both much better than the old mysql extension - they support prepared statements for example (which is better and more secure than escaping). Commented Jun 9, 2012 at 16:50
  • Okay so I've taken all your advice and I'm looking into OOP. Thanks for all the help! Commented Jun 11, 2012 at 20:31

3 Answers 3

Reset to default
3

In your specific case you should declare global within function all variables outside the function. So

function dbQuery($database, $reqquery){
    global $px_host,$px_dbuser,$px_dbpass;
    // rest of function
}

But your code can be improved: You should define constants at the beginning of your script

define('DB_HOST','localhost');
define('DB_USER','user');
define('DB_PASS','pass');

and use constants inside the function (so no need to declare global, and it's more logic because host, user and pass aren't variable but constants).

You should connect at database only once at the beginning of your flow so the function dbQuery execute only the query (according with the function name).

EDIT for completeness of answer:

As some users say you in other comments, I invite you to read the php doc for mysql_connect and see the red advise:

Use of this extension is discouraged. Instead, the MySQLi or PDO_MySQL extension should be used. See also MySQL: choosing an API guide and related FAQ for more information. Alternatives to this function include:

I'm not here for say you what you MUST do in your project, but read the doc and follow the tipps/suggestions is essential for the success of your project. :)

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

It's getting late, so this is only a partial answer.

Another approach you could take is to pass the database instance into your helper function, thus resolving the credentials issue.

function dbQuery($database, $reqquery)
{
    if (false !== ($query = mysql_query($reqquery, $database))) {
        exit("Error - query error.");
    }

    return $query;
}

Now this function receives its dependency via the arguments and is also a lot shorter and doesn't connect / query / disconnect every time.

The remaining code has to be written elsewhere; if you require a database at every page, you can write this pretty high up the chain:

if (false === ($connect = mysql_connect($px_host, $px_dbuser, $px_dbpass))) {
    exit("Error - cannot connect to MySQL server - " . mysql_error());
}

if (false === mysql_select_db($database)) {
    exit("Error - cannot select database - " . mysql_error());
}

Then you pass $connect around wherever it's required.

$res = dbQuery($connect, 'SELECT "hello world"');

Of course, mysql_connect and friends use implied connections, so you technically don't need to pass it around anyway; it's a good pattern nonetheless.

Last but not least

Learn how to use PDO and revel in the magical world of OOP ;-)

Improve this answer

Comments

-3

If you set the variable global, you will need to set it global in the function as well. In this case:

$px_host = "localhost";

function dbQuery($database, $reqquery){
    global $px_host;
    if(!$connect = mysql_connect($px_host, $px_dbuser, $px_dbpass)){
        exit("Error - cannot connect to MySQL server - " . mysql_error());
    }

    if(!$database = mysql_select_db($database)){
        exit("Error - cannot select database - " . mysql_error());
    }

    if(!$query = mysql_query($reqquery)){
        exit("Error - query error.");
    }

    return $query;
}

This should fix this problem.

Improve this answer

5 Comments

A global statement outside a function does nothing - you only need it in a function to "import" a global variable.
That's right, in essence, but you shouldn't use "global" on the variable outside the function. It's not used to declare global variables; they are global by default.
-1 You managed to make his code worse. Instead of providing a sensible solution, you chose to add global state.
I wasn't going to downvote originally, even though subsidizing the use of global at all should be discouraged and result in a downvote. But then I saw that you also said nothing about the OP's use of the mysql_* functions. The PHP manual explicitly says DO NOT USE mysql_* FUNCTIONS. Don't encourage global and don't encourage mysql_*. -1
This is here because I need to leave a comment.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.