1
\$\begingroup\$

I have written a function which fetches all comments for a given movie. Since comments can be nested (e.g. someone replied to a comment) I am fetching them recursively (based on the parent comment id). Currently, my function looks like this:

<?php
$movieid = $this->movieid;

        function display_comments($movieid, $comment_parent_id=0, $level=0) {

        if(!$comments = CommentModel::FetchAllComments($movieid, $comment_parent_id)){
            return false;
        }
          foreach($comments as $comment) {
                $comment_id = $comment->comment_id;
                $user_id = $comment->user_id;
                $comment_content = $comment->comment_content;
                $comment_timestamp = $comment->comment_creation_datetime;
                $comment_is_deleted = $comment->comment_is_deleted;
                $comment_parent_id = $comment->comment_parent_id;

      ?>
          <div class="comment">
              <b><a href="<?php echo Config::get('URL'); ?>profile/showProfile/<?php echo $user_id; ?>"><?php echo UserModel::getPublicProfileOfUser($user_id)->user_name; ?></a>, <?php echo $comment_timestamp; ?></b> <br>

          <?php echo $comment_is_deleted == 0 ? $comment_content : '<i>Deleted</i>' ?><br>

            <form action="<?php echo Config::get('URL'); ?>comment/addComment" method="post">
            <div class="form-group">
            <label>
                Comment: <input type="text" class="form-control" name="comment_contet" required />
            </label>
            </div>

            <!-- set CSRF token at the end of the form -->
            <input type="hidden" name="csrf_token" value="<?= Csrf::makeToken(); ?>" />
            <input type="hidden" name="movie_id" value="<?php echo $movieid; ?>" />
            <input type="hidden" name="parent_comment" value="<?php echo $comment_id; ?>">

            <div class="form-group">
            <input type="submit" class="btn btn-default" value="Submit" />
            </div>
        </form>

              <?php if(Session::get("user_account_type") == 7) : ?> 
              <a href="<?php echo Config::get('URL').'comment/softDelete/'.$comment_id.'/'. $movieid ?>"><?php echo $comment_is_deleted == 0 ? 'Delete' : 'Undelete' ?></a>
              <?php endif;

        //Recurse
        display_comments($movieid, $comment_id, $level+1); ?>
     </div> <!-- /comment -->
   <?php }

}
    display_comments($this->movieid);
?>

It is working, but it seem very ugly!

Right now, I'm not fetching anything from the controller apart from the $movieid.

A thought:

Is there any way to change this function, so the controller fetches the comments:

public function view($movieid)
{
    $this->View->render('comment/view', array(
        'comments' => CommentModel::FetchAllComments($movieid),
        'movieid' => $movieid
    ));
}

And then a foreach function can output the comments, recursively, something like (very sorry for pseudo code):

    foreach($this->comments as $comment){
echo '<div class="comment">';
       if($comment->comment_parent_id){
           foreach($comment->comment_parent_id as $comment){
             echo '<div class="comment">'.$comment->comment_content . 'this is a parent comment</div>';
           }
        }
    echo $comment->comment_content . 'the "main" comment</div>';
    }

Now display_comments is running itself over and over, wouldn't it be correct to fetch all comments, then arrange them in the right order?

My question:

How can i improve my recursive comment function?

\$\endgroup\$
  • \$\begingroup\$ Just to confirm, you aren't asking for new functionality that isn't already working, right? Just how to improve it? \$\endgroup\$ – Insane Jan 23 '16 at 23:20
  • \$\begingroup\$ I would like to improve my function, because it is not satisfying to look at. \$\endgroup\$ – Adam Jan 23 '16 at 23:28
  • \$\begingroup\$ I have reworded your question to make it a bit clearer as to what it is doing and that it is indeed working as intended - you just like to improve it. \$\endgroup\$ – ChrisWue Jan 24 '16 at 6:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.