Re: Re: creating a threaded message system--sorting messages

Ben Liu wrote:

Thanks to everybody who posted on this thread. I wanted to post back the solution I came up with after removing the DB query from the recursion. In case anyone else is trying to accomplish the same thing, this is how I solved it (criticisms welcome, and note that I have not tested it extensively yet): DB structure table: discussion_posts post_id (auto increment, unique, primary index) parent_id (post_id corresponding to this post's parent) discussion_id (to allow multiple discussions, not required but I added a discussions member_id (to identify the post to the particular member posting the post) table as well to store the discussion description and unique ID for each discussion) dt_posted (date/time the post was posted) subject (title of the particular post) post_text (substance of the post) So the user selects a particular discussion from a list of discussions. We query the database for the post_id of the first post in the discussion requested, this post will have a parent_id of '0' or NULL since it has no parent. We then query the DB for all the posts in the discussion joining the members table using member_id to grab the member's first and last name (or any other member info desired). We order this query info by dt_posted. We then write the contents of our second query into a two-dimensional array ($workArr): while ($row=@mysql_fetch_array($result)) { foreach ($row as $key => $value) { if ($key=="post_id") $numerKey=value; $workArr[$key][$numerKey]=$value; } } The processing of the threads into proper order looks like this:

    function processthread($post_id, $workArr) {
        echo "<ul class=\"posting\">";
        echo "<h3>{$workArr['subject'][$post_id]} (#{$post_id})</h3>\n";
        echo "<h4>by {$workArr['first_name'][$post_id]}

{$workArr['last_name'][$post_id]} "."&bull; on " . fixdate($workArr['dt_posted'][$post_id]) . "</h4>";

        echo "<li>{$workArr['post_text'][$post_id]}</li>\n";
        echo "<h5>reply to this</h5>";
            // find all children, call itself on those too
        foreach ($workArr['post_id'] as $value) {
            if ($workArr['parent_id'][$value]==$post_id) {
            processthread($workArr['post_id'][$value], $workArr);
            } // end if
        } // foreach
        echo "</ul>";
    }

And somewhere in the HTML, where appropriate, the function processthread is called for the first time passing it the $post_id value we determined in the first query (the very first post of the discussion) and the $workArr array. Use of unordered lists for design/layout is helpful here since browsers will by default indent lists within lists and list items within their respective list. This saves some PHP and CSS troubles if one were to use <div> or <p> structure instead, as I found out. Apologies for any formatting issues and vagaries about the data structure and variable naming in this post. - Ben

I think that will work, but there is one huge improvement you can make. You are making millions of copies of your work array! Not only do you pass the entire array by value recursively, but foreach makes copies of the arrays it loops through! So who knows how many copies of the darned work array are being spawned left and right. This can be a memory leak, consuming lots of memory until it finishes the recursion. The first thing you can do is tell the function the array is to be handled by reference: function processthread($post_id, &$workArr) { You never change the work array, I think, so you can just keep referring back to the original. The second thing to do is to make the foreach work on references instead of copies. I actually think that since what we have in our function is now a reference, the foreach WILL NOT copy it. But if it still does, there is always the fallback of doing a foreach on array_keys($workArr). Then each $value would be the key, which you'd use to reference $workArr. Regards, Adam.