7

I'm new to PHP, so I'm not exactly sure how it works.

Anyway, I would line to return a multidimensional array to another method, essentially something storing a small amount of record and columns, table like structure.

I've written the following, no warning but no data either

public function GetData($sqlquery)
{
    include 'config.php';

    $result = mysql_query($sqlquery,$con);
    $data = array();

    while($row = mysql_fetch_assoc($result))
    {
        $data[] = $row;
    }

    return $data;
}

Most likely doing something stupid

Help appreciated.

EDIT:

Thanks for all the fast replies

I figured out why this wasn't working, I was addressing the array as such

print $data[0][0];

Rather than

print $data[0]['title']; 

for example, thanks all :)

PS I really find it hard to believe you can't say $data[0][5], It's more logical IMO than specifying a string value for location

4
  • Do you create $con in the config.php? Commented Mar 19, 2011 at 23:01
  • Can you say what's certainly wrong with this code? Commented Mar 19, 2011 at 23:08
  • Did you read the answers and correct your other mistakes too? Commented Mar 19, 2011 at 23:34
  • I really find it hard to believe that people do not read manual page for the function they're using. mysql_fetch_assoc is your own choice. if you wanted numeric index instead of reliable textual one, you had to choose another function that suits your needs Commented Mar 19, 2011 at 23:44

3 Answers 3

5

Your code seems okay. At least, you're going in right direction.

Just some minor corrections:

  • NEVER include config inside of a function. it should be done in class constructor
  • if you really want to use connection identifier - make it class variable. But for most applications using single connection to db its unnecessary to use $con, so you can omit it
  • error handling is absolutely necessary

so,

public function GetData($sqlquery)
{
    $data = array();
    $result = mysql_query($sqlquery) or trigger_error(mysql_error().$sqlquery);
    if ($result)
    {
        while($row = mysql_fetch_assoc($result))
        {
            $data[] = $row;
        }
    }
    return $data;
}

run this code and see what it says.

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

1 Comment

Many thanks for your tips, I've solved the problem though, I was addressing the array incorrectly
1

Maybe I'm wrong, if all this happens in config.php, but I think, yu miss a few steps:

Create connection:

$con = mysql_connect("localhost", "mysql_user", "mysql_password");

Select database:

mysql_select_db("mydbname");

After this, comes the mysql_query. But you say there are no warnings, so I assume, you do all this.

I would do something like this (there are better, more complex solutions):

include 'config.php'; // contains definition of $conf array

$con = mysql_connect($conf['host'], $conf['user'], $conf['pass']);
mysql_select_db($conf['db']);

function GetData($sqlquery)
{
    global $con;

    $result = mysql_query($sqlquery,$con);
    $data = array();

    while($row = mysql_fetch_assoc($result))
    {
        $data[] = $row;
    }

    return $data;
}

Comments

1

If you used the mysqli extension instead of mysql you could use fetch_all() which is faster than filling the array in a loop. So your function only needs to return the result of fetch_all()

return $result->fetch_all(MYSQLI_ASSOC);

Script

<?php

ob_start(); 

try
{
    $db = new mysqli("localhost", "foo_dbo", "pass", "foo_db", 3306);

    if ($db->connect_errno) 
        throw new exception(sprintf("Could not connect: %s", $db->connect_error));

    $sqlCmd = "select * from users order by username";

    $startTime = microtime(true);

    $result = $db->query($sqlCmd);

    if(!$result) throw new exception(sprintf("Invalid query : %s", $sqlCmd));

    if($result->num_rows <= 0){
        echo "no users found !";
    }
    else{

        $users = $result->fetch_all(MYSQLI_ASSOC); //faster 

        //while($row = $result->fetch_assoc()) $users[] = $row; //slower

        echo sprintf("%d users fetched in %s secs<br/>", 
            count($users), number_format(microtime(true) - $startTime, 6, ".", ""));

        foreach($users as $u) echo $u["username"], "<br/>";
    }
    // $result->close();
}
catch(exception $ex)
{
    ob_clean(); 
    echo sprintf("zomg borked - %s", $ex->getMessage());
}
//finally
if(!$db->connect_errno) $db->close();
ob_end_flush();
?>

Testing

//fetch_all()

 1000 users fetched in 0.001462 secs
 5000 users fetched in 0.005493 secs
15000 users fetched in 0.015517 secs
50000 users fetched in 0.051950 secs
100000 users fetched in 0.103647 secs

//fetch_assoc plus loop

 1000 users fetched in 0.001945 secs
 5000 users fetched in 0.008101 secs
15000 users fetched in 0.023481 secs
50000 users fetched in 0.081441 secs
100000 users fetched in 0.163282 secs

5 Comments

that's useless tests. 0.0002 will never be a bottleneck.
did i say it would be - or was it simply just more efficient and elegant ?
there is not much elegance as the code will be hidden in the helper function code. as for the efficiency, it's not that much to notice. You shouldn't use this function to process more than hundred rows anyway
It's inefficient because you're benchmarking where it is not needed.
so as it's not much to notice i should use a loop - thanks, will do in the future.

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.