views:

159

answers:

2

I have a MySQL singleton class written in PHP. Its code is listed below:

class Database {
    private $_result = NULL;
    private $_link = NULL;
    private $_config = array();
    private static $_instance = NULL; 

    // Return singleton instance of MySQL class
    public static function getInstance(array $config = array()) {
        if (self::$_instance === NULL) {
            self::$_instance = new self($config);
        }

        return self::$_instance;
    }

    // Private constructor
    private function __construct(array $config) {
        if (count($config) < 4) {
            throw new Exception('Invalid number of connection parameters');  
        }

        $this->_config = $config;
    } 

    // Prevent cloning class instance
    private function __clone() {}

    // Connect to MySQL
    private function connect() {
        // Connect only once
        static $connected = FALSE;

        if ($connected === FALSE) {
            list($host, $user, $password, $database) = $this->_config;

            if ((!$this->_link = mysqli_connect($host, $user, $password, $database))) {
                throw new Exception('Error connecting to MySQL : ' . mysqli_connect_error());
            }

            $connected = TRUE;

            unset($host, $user, $password, $database);      
        }
    } 

    // Perform query
    public function query($query) {
        if (is_string($query) and !empty($query)) {
            $this->connect();
            if ((!$this->_result = mysqli_query($this->_link, $query))) {
                throw new Exception('Error performing query ' . $query . ' Message : ' . mysqli_error($this->_link));
            }
        }
    }

    // Fetch row from result set
    public function fetch() {
        if ((!$row = mysqli_fetch_object($this->_result))) {
            mysqli_free_result($this->_result);
            return FALSE;
        }

        return $row;
    }

    // Get insertion ID
    public function getInsertID() {
        if ($this->_link !== NUlL) {
            return mysqli_insert_id($this->_link); 
        }

        return NULL;  
    }

    // Count rows in result set
    public function countRows() {
        if ($this->_result !== NULL) {
           return mysqli_num_rows($this->_result);
        }

        return 0;
    }  

    // Close the database connection
    function __destruct() {
        is_resource($this->_link) AND mysqli_close($this->_link);
    }    
}

I also have this recursive function which must returns a full category tree (the SQL table and its content can be found here):

function getCategories($parent = 0) {
    $html = '<ul>';    
    $query = "SELECT * FROM `categories` WHERE `category_parent` = '$parent'";
    $database->query($query);    
    while($row = $database->fetch()) {
        $current_id = $row->category_id;
        $html .= '<li>' . $row->category_name;
        $has_sub = 0;
        $query = "SELECT `category_parent` FROM `categories` WHERE `category_parent` = '$current_id'";
        $database->query($query);
        $has_sub = $database->countRows();

        if ($has_sub > 0) {        
            $html .= getCategories($current_id);
        }

        $html .= '</li>';
    }

    $html .= '</ul>';
    return $html;
}

Now, the problem is that the function returns only 3 categories, not the full tree. I rewrote the function using plain MySQL functions (mysql_query(), mysql_fetch_object() etc.) and it returns the right result.

So my conclusion in that there's something wrong with that class. Note that I've been using this class in most of my projects, but never had this issue.

Any idea what?

Thanks.

EDIT: Trying to make it to return an associative array

function getCategories($parent = 0) {
    global $database;
    $categories = array();

    $query = "SELECT * FROM `categories` WHERE `category_parent` = '$parent'";
    $database->query($query);    
    while($row = $database->fetch()) {
        $categories[] = array('id' => $row->category_id, 'name' => $row->category_name);        
    }

    for ($i = 0; $i < count($categories); $i++) {

        $categories[$i]['id']['children'] = getCategories($categories[$i]['id']);

    }

    return $categories;
}

The code above returns the following array, but is not quite ok:

Array
(
    [0] => Array
        (
            [id] => 1
            [name] => Categoria 1
            [children] => Array
                (
                    [0] => Array
                        (
                            [id] => 4
                            [name] => Categoria 1.1
                            [children] => Array
                                (
                                    [0] => Array
                                        (
                                            [id] => 7
                                            [name] => Categoria 1.1.2
                                            [children] => Array
                                                (
                                                )

                                        )

                                )

                        )

                    [1] => Array
                        (
                            [id] => 5
                            [name] => Categoria 1.2
                            [children] => Array
                                (
                                )

                        )

                    [2] => Array
                        (
                            [id] => 6
                            [name] => Categoria 1.3
                            [children] => Array
                                (
                                )

                        )

                )

        )

    [1] => Array
        (
            [id] => 2
            [name] => Categoria 2
            [children] => Array
                (
                )

        )

    [2] => Array
        (
            [id] => 3
            [name] => Categoria 3
            [children] => Array
                (
                )

        )
)
+3  A: 

The problem that you're seeing is that you a running other queries over your first query. So after you recurse through the first branch the function fails because it goes back to the original which loop and finds that its already at the end of the resulting rows from the first branch (3 levels of recursion in).

A quick fix would be to rewrite your function using two loops instead of just one. You can also optimise the function this way too by reducing the number of DB calls you have to make.

Your first query is fine. But in your while loop, just grab the appropriate id and name elements and store them in an array. Then reloop through the array that you just created to recurse. Also, you can eliminate several queries by not running a "check" query. Just recurse - and if there are no elements the id:name array - return an empty string.

EDIT: an example (untested)

function getCategories($parent = 0) {
    $categories = array();
    $html = "<ul>";
    $query = "SELECT * FROM `categories` WHERE `category_parent` = '$parent'";
    $database->query($query);    
    while($row = $database->fetch()) {
        $categories[$row->category_id] = $row->category_name;
    }
    foreach($categories as $cid=>$category) {
        $html .= "<li>{$row->category_name}";
        $inner = getCategories($cid);
        if($inner != "<ul></ul>")
            $html .= $inner;
        $html .= "</li>"
    }
    $html .= "</ul>";
    return $html;
}
thetaiko
@thetaiko: I understand what you're saying, but I'm not sure about writing the changes. Can you provide an example please?
Psyche
@Psyche- see my edited answer. I haven't tested this and there may be errors, but it conveys the general idea. Also, I generally don't like mixing HTML into my PHP, but did so for the sake of this example. You could expand on this to run another query that would check for sub-categories for all the keys in `$categories` at once, but I omitted that as well to keep it simple.
thetaiko
@thetaiko: there was a little error, but it's ok now. It works! I don't like mixing HTML into PHP either, so I was thinking about returning this as an associative array and then display it with Smarty.
Psyche
@Psyche - good idea!
thetaiko
@thetaiko: do you think we can modify this function in order to return an associative array? Also it must save both category_name and category_slug.
Psyche
@Psyche - indeed you can. Just get rid of the `$html` variable and return your `$categories` array instead. You can add both the `category_name` and `category_slug` in the first `while` loop. Instead of using `$inner` just set `$category[$cid]['children'] = getCategories($cid);`
thetaiko
@thetaiko, can you help me with an example? I seem to fail in making it work.
Psyche
@Psyche - show me what you've got so far and I'll help you work through it.
thetaiko
@thetaiko: I made an edit to my question. The code is there. I can't make it with that code inside the for loop.
Psyche
@Psyche - What exactly isn't working? I would suggest changing the following line: `$categories[$i]['id']['children'] = getCategories($categories[$i]['id']);`. Remove the `['id']` so that it creates another element in the category array instead of overwriting the 'id' element.
thetaiko
The new line should read: `$categories[$i]['children'] = getCategories($categories[$i]['id']);`
thetaiko
Then do a `print_r` on the results of the first getCategories call to see the tree structure being returned.
thetaiko
@thetaiko: the output is not quite ok. I have edited my post again with the output.
Psyche
The output looks right to me - what is wrong with it? What structure are you trying to achieve?
thetaiko
My bad. It's ok. Thanks.
Psyche
A: 

Your basic problem is that your database object is schizophrenic: it's both a connection object and a resource object. Additional queries made before the the previous one is finished will overwrite the existing resource handle.

The solution is simple: just create a second class which encapsulates the resource handles and returns the data.

staticsan