Strange for loop / array problem in PHP

Question

This is my first question here on StackOverflow, and quite frankly I'm fairly new to PHP. Just to give you a brief heads-up ;)

I'm building an OOP-based website, in a 3-tier architecture. In my data abstraction layer, I have an object I called DbAdapter, which contains all functions necessary to communicate with the database. One of these functions is the one below: read($sql), which takes an SQL query and stores the result in a two-dimensional array.

For this, it uses two nested for-loops (one for the rows and one for the columns per row). And while the iterator $i increments as usual, somehow the last element of the array is overwritten.

I have absolutely no idea how this is possible, so the mistake I made must be extremely stupid.

Anyone care to help out a newbie?

Thanks in advance, Sam

public $loadedRows;
public function read($sql)
{
    if ($this->connect())
    {
        $result = mysql_query($sql);
        if ($result)
        {
            $totalRows = mysql_num_rows($result);
            $totalFields = mysql_num_fields($result);

            for ($i = 0; $i < $totalRows; $i++)
            {
                for ($j = 0; $j < $totalFields; $j++)
                {
                    $fieldName = mysql_field_name($result, $j);
                    $loadedFields["$fieldName"] = mysql_result($result, $i, $fieldName);
                }

                $this->loadedRows[i] = $loadedFields;
            }

            $this->closeConnection();
            return $this->loadedRows;
        }
    }
}

Answer 1

you just forgot $ before i $this->loadedRows[$i]

and this code should be way shorter:

public function read($sql)
{
    $a = array();
    $result = mysql_query($sql);
    if ($result)
    {
        while($row = mysql_fetch_assoc($res)) $a[]=$row;
    }
    return $a;
}

That's ALL.

and to catch these errors yourself, you should set error reporting level to E_ALL
to do that you can add this line

error_reporting(E_ALL);

in your config file.
It will tell PHP to watch such mistakes (using undefined constant i in this case) and notify you

Answer 2

Any reason you can't just do:

while($row = mysql_fetch_assoc()) {
    $this->loadedRows[] = $row;
}

In place of both your loops? Fetching fieldnames/values individually like this is hideously slow compared to just fetching a associative row which has the fieldname/values in it already.

Personally, I prefer eating a bowlful of cheerios, rather than individually pulling one from a box, putting it in the bowl, pouring on a drop of milk, eating it, then going back to the box for another one.

Answer 3

My guess is the missing $ before i here is your culprit:

           $this->loadedRows[i] = $loadedFields;

Answer 4

If I were you, I'd use PDO instead of these old mysql functions. PDOStatement::fetchAll does what you need.