Sunday, July 10, 2011

Avoid Fetch-Object Abuse

Lately I'm finding a lot of instances of the mysql_fetch_object() function being used in a particular codebase I help maintain. Unfortunately, I've yet to see it used correctly. It always seems to be used to retrieve a stdClass object from a query result where mysql_fetch_array() or mysql_fetch_assoc() would be the more appropriate choice.
$row = mysql_fetch_object($result);
$kitten = new Kitten();
$kitten->setName($row->name);
$kitten->setColor($row->color);
...
Put aside the argument that the code should be using PDO or the MySQLi extension instead of the legacy MySQL extension. mysqli_result::fetch_object() and PDOStatement::fetchObject() have the same potential for abuse. The above code is wrong because the returned result is an object but treated like it's an array.

The mysql_fetch_array() and mysql_fetch_assoc() functions are used to retrieve a row from the query's result set. I prefer to use mysql_fetch_assoc() so I can access the values using column names.
  • mysql_fetch_array() - return an array corresponding to the fetched row and move the internal data pointer to the next. The array is indexed numerically if MYSQL_NUM is specified, associatively if MYSQL_ASSOC is specified, or both by default or if MYSQL_BOTH is specified.
  • mysql_fetch_assoc() - return an associative array corresponding to the fetched row and move the internal data pointer to the next. It is the same as calling mysql_fetch_array() using MYSQL_ASSOC.
mysql_fetch_object() on the other hand was designed to create and populate an instance of an object using the row of data.
$kitten = mysql_fetch_object($result, "Kitten");
If mysql_fetch_object() is called without the second argument, the name of the object type to create, then a stdClass object is returned that has public properties named after the result columns to expose the data. There isn't any benefit to this over an array. It is just an object for the sake of having an object, and not a very useful object at that. Incidentally, an array can always be casted to a stdObject later if, for some odd reason, it's needed:
$obj = (object)$array;
An object is a data structure that encapsulates variables to maintain state and related functions to manipulate the state. A stdClass data object just collects values and then exposes them as public properties. It does not observe proper encapsulation, there is no state, and there are no methods to interact with the object. It is no more than an array that uses object-syntax. My rule of thumb is: if you call mysql_fetch_object() without specifying a class name, you're "doing it wrong."

A related point worth mentioning is even a good programmer may run into some difficulty when trying to use mysql_fetch_object() correctly since the function is intrinsically broken (though some will argue differently). The function first creates an instance of the specified class, then populates its internal variables, and lastly invokes the constructor. The purpose of a constructor is to initialize the object's values and resources, but since the constructor is called last the values set from the result row may be overwritten. The programmer must take this into account and guard against it if the objects might be instantiated by both mysql_fetch_object() and new.
class Kitten
{
    private $name;
    private $color;
    ...
    public function __construct() {
        if (!isset($this->name)) {
            $this->name = "Unknown";
        }
        ...
If you're like me and have a large codebase with the misuse of mysql_fetch_object() deeply entangled throughout, it may not be practical to find and fix each instance. The best advice I can offer is to educate yourself and others how the function should be used so its abuse isn't perpetuated. Then, be cautious when using mysql_fetch_object() correctly and understand the process it follows to create and return an object. If not for yourself, then do it for the kittens.

Every time you abuse mysql_fetch_object() God kills a kitten!

4 comments:

  1. This echoes my first thoughts when running into this "pattern" across Drupal code. It later turned into "well, it's easier to write this way" and never gave it a secound thought.

    My first thought *now* would be: maybe there *is* a reason? My very unscientific search came up with this discussion in StackOverflow: http://stackoverflow.com/questions/124240/mysql-results-in-php-arrays-or-objects

    Perhaps you meant *overkill* instead of "abuse"? It seems there is no performance penalty for either function.

    ReplyDelete
  2. The point about constructors and fetch object is important (and relevant to PDO::fetch also), but there is a simpler way set (scalar) default values, that is:

    class Kitten
    {
    private $name = 'Unknown';
    private $color;
    ....
    }

    ReplyDelete
  3. When I've been developing in a system that requires the use of mysql_fetch_xxx, I've generally used the object version since I prefer the '->param' syntax to '["param"]'. I didn't realise you could specify a class to populate, so thats very useful thanks!

    ReplyDelete
  4. Yes, I'm "picking" on the mysql_ variant here, but it applies to both mysqli and PDO object functions.

    An argument could be made that setting values at their declaration like that instead of the constructor is "bad practice". But that's so widespread that people would probably come after me with pitchforks and torches if I went down that road.

    Thanks for the comments!

    ReplyDelete