4

I'm trying to create a series of <ul> and <li> to create a directory/file structure to navigate some files created from a table in a dB. The table (tb_lib_manual) contains both files and folders.

If a record has a null entry for fileID then it is a folder not a file. each record has a parentID to show which folder is parent, for files and folders in the root this is 0.

The php code is thus:

class library_folders extends system_pageElement 
{
    private $html = '';
    private $i = 0;
    private $stmtArray = array();
    private $objectArray = array();

    function __construct() 
    {
        parent::__construct();
        $this->nextList();
    }


    function nextList($parentID = 0) 
    {
        $qSQL = 'SELECT * FROM tb_lib_manual WHERE parentID=:parentID';
        $stmtArray[$this->i] = $this->dbConnection->prepare($qSQL);
        $stmtArray[$this->i]->bindValue(':parentID', $parentID, PDO::PARAM_INT);
        $stmtArray[$this->i]->execute();
        if($stmtArray[$this->i]->rowCount() > 0)
        {
            $display ='';
            if($parentID != 0)
            {
                $display = ' style="display:none"';
            }
            $this->html .= '<ul' . $display . '>';
        }


        while ($this->objectArray[$this->i] = $stmtArray[$this->i]->fetchObject()) 
        {
            $this->html .= '<li>' . $this->objectArray[$this->i]->title;
            if($this->objectArray[$this->i]->fileID == null)
            {
                //we have a folder!
                $manualID = $this->objectArray[$this->i]->manualID;
                $this->i ++;
                $this->nextList($manualID);
                $this->i--;
            }
            $this->html .= '</li>';
        }


        if($stmtArray[$this->i]->rowCount() > 0)
        {
            $this->html .= '</ul>';
        }   

        echo $this->html;
    }   
function __destruct()
    {
        parent::__destruct();
    }

}

The problem is when the code returns back to the while loop after calling itself it restarts the loop rather than carrying on where it left off, causing a repeat in the child folder. Is there either a better way to do this or am I doing something wrong!?

Table looks like this:

enter image description here

output like this: '

  • A Manual
  • A Manual
  • Folder 1
  • Nasa exercise
  • A Manual
  • A Manual
  • Folder 1
  • Nasa exercise
'

8
  • is manualID the same as ParentID? Commented Jun 8, 2015 at 12:07
  • yes it is sorry should've explained, obviously different columns... Commented Jun 8, 2015 at 12:09
  • You might place some debug output of the variables like var_dump($manualID);. Commented Jun 8, 2015 at 12:17
  • Hi sorry have no idea how to place output of debug as I can't get my debug to work (aptana studio 3) however the table data looks like this (just edited question) Commented Jun 8, 2015 at 12:27
  • I'm more interested in what's in $this->objectArray. I think we're missing the other important parts of the class Commented Jun 8, 2015 at 12:32

2 Answers 2

1

Fixed the code, very silly mistake, echo in the wrong place...

class library_folders extends system_pageElement 
{
    private $html = '';
    private $i = 0;
    private $stmtArray = array();
    private $objectArray = array();

    function __construct() 
    {
        parent::__construct();

        $this->nextList();

        echo $this->html;

    }


    function nextList($parentID = 0) 
    {
        $qSQL = 'SELECT * FROM tb_lib_manual WHERE parentID=:parentID';
        //echo $this->i;        
        $stmtArray[$this->i] = $this->dbConnection->prepare($qSQL);
        $stmtArray[$this->i]->bindValue(':parentID', $parentID, PDO::PARAM_INT);
        $stmtArray[$this->i]->execute();
        if($stmtArray[$this->i]->rowCount() > 0)
        {
            $this->html .= '<ul>';
        }

        while ($this->objectArray[$this->i] = $stmtArray[$this->i]->fetchObject()) 
        {
            $this->html .= '<li>' . $this->objectArray[$this->i]->title;
            if($this->objectArray[$this->i]->fileID == null)
            {
                //we have a folder!
                $manualID = $this->objectArray[$this->i]->manualID;
                $this->i ++;
                $this->nextList($manualID);
                $this->i--;
            }
            $this->html .= '</li>';
        }


        if($stmtArray[$this->i]->rowCount() > 0)
        {
            $this->html .= '</ul>';
        }   

    }   

    function __destruct()
    {
        parent::__destruct();
    }

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

Comments

0

Yes, there are better ways of doing this.

If you fetch the whole tree every time, you might as well load all the nodes into a big array (of objects), put each node's id as index and then loop over the array once to create the parent references by for example

// create a fake root node to start your traversal later on
// only do this if you don't have a real root node 
// which I assume you don't
$root = (object) ["children" => []];

// loop over all nodes
foreach ($nodes as $node)
{
    // if the node has a parent node that is not root
    if ($node->parentId > 0)
    {
         // put it into it's parent's list of children
         $nodes[ $node->parentId ]->children[] = $node;
    }
    else
    {
         // otherwise put it into root's list of children
         $root->children[] = $node;
    }
}

Complexity: You do one query and you have to iterate all your nodes once.

For this to work your nodes need to be objects. Otherwise each assignment to $node->children will create a copy of the assigned node where you wanted a reference.

If you do not want to fetch all nodes, you can go through your tree level by level by creating a list of node ids from the previous level.

function fetchLevel ($nodes, $previousLevelIds)
{
    // get all children from the previous level's nodes
    // I assume $previousLevelIds to be an array of integers. beware of sql injection
    // I refrained from using prepared statements for simplicity
    $stmt = $pdo->query("SELECT id, parentId FROM nodes WHERE parentId IN (".implode(",",$previousLevelIds).")");

    // fetch nodes as instances of stdclass
    $currentLevelNodes = $stmt->fetchAll(PDO::FETCH_OBJ);

    $nextLevelIds = [];
    foreach ($currentLevelNodes as $node)
    {
         // ids for next level
         $nextLevelIds[] = $node->id;

         // parent <-> child reference
         $nodes[ $node->parentId ]->children[] = $node;
    }

    // fetch the next level only if we found any nodes in this level
    // this will stop the recursion
    if ($nextLevelIds)
        fetchLevel($nodes, $nextLevelIds);
}

// start by using a fake root again
$root = (object) ["id" => 0, "children" => []];
$nodes = [0 => $root];
fetchLevel($nodes, [0]);

// or start with a specific node in the tree
$node = $pdo->query("SELECT id, parentId FROM nodes WHERE id = 1337")->fetch(PDO::FETCH_OBJ);
$nodes = [$node->id => $node];
fetchLevel($nodes, [$node->id]);

// or a number of nodes which don't even have to 
// be on the same level, but you might fetch nodes multiple times
// if you it this way

Complexity: Number of queries <= height of your tree. You only iterate each fetched node once.

For displaying the tree as html list you iterate once more:

class Foo {

    public $html;

    public function getList ($nodes)
    {
         // outer most ul
         $this->html = '<ul>';

         $this->recurseList($nodes);

         $this->html .= '</ul>';
    }

    protected function recurseList ($nodes)
    {
        foreach ($nodes as $node)
        {
            $this->html .= "<li><span>".$node->name."</span>";

            if ($node->children)
            {
                if ($node->parentId > 0)
                    $this->html .= '<ul style="display:none">';
                else
                    $this->html .= '<ul>';

                $this->recurseList($node->children);

                $this->html .= "</ul>";
            }

            $this->html .= "</li>";
        }
    }
}

Some unrelated remarks:

  • instead of a hard-coded style="display:none" you could just use a css rule like ul li ul {display:none} to hide all lists below root
  • I split fetching the data from the database and converting the data for displaying into two separate scripts, which is the standard when you develop using MVC. If you don't want to do this run the scripts in succession.
  • PHP itself is a template engine, but consider using another template engine to display your html like Smarty. I personally prefer smarty for it's simpler syntax.
  • stackoverflow answer on how to bind a PHP array to a MySQL IN-operator using prepared statements
  • if you need to fetch subtrees often consider using a special table structure to reduce the number of queries

2 Comments

Thanks Basti, fixed my code, but I think I'm starting to see why your solution is better, but I don't understand it fully yet as I'm a novice...
If you are interested to learn and have questions, just ask away. Since you found the problem in your code yourself, put your own answer explaining it and accept it. ;-)

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.