Recursive comment function echoes HTML inside function

Question

To display comments recursively I'm using this:

function fetch_article_comments($article_id, $parent_id) {
$app = new Connection();

    if ($parent_id <= 0) {
        $parent_id = NULL;
    }
    $sql = "SELECT * FROM recursive WHERE article_id = :article_id AND comment_parent <=> :parent_id ORDER BY comment_timestamp ASC";
    $query = $app->getConnection()->prepare($sql);

    $query->execute(array(':article_id' => $article_id,
                          ':parent_id' => $parent_id));

    return $query->fetchAll();
}

And to display the comments:

function display_comments ($article_id, $parent_id=0, $level=0) {
$comments = fetch_article_comments($article_id, $parent_id);
  foreach($comments as $comment) {
        $comment_id = $comment->comment_id;   
        $member_id = $comment->member_id;
        $comment_text = $comment->comment_text;
        $comment_timestamp = $comment->comment_timestamp;  //get timeAgo


        // RENDER
      ?>
          <div class="comment">
              <b><?= timeAgoInWords($comment_timestamp); ?></b><br>
          <?= $comment_text ?><br>
              <a href="#" class="replyLink" data-comment-id="<?= $comment_id ?>"><span class="uk-icon-comment-o"> reply</span></a>
              <a href="recursive.php?delete=<?= $comment_id ?>"><span class="uk-icon-trash-o"> delete</span></a>


    <form action="recursive.php" method="post" class="uk-form comment_form" id="<?= $comment_id ?>">
    <fieldset data-uk-margin>
        <input type="hidden" name="id" value="<?= $article_id ?>" />
        <input type="hidden" name="parent" value="<?= $comment_id ?>" />
        <input type="text" placeholder="Comment" name="svar">
        <button class="uk-button">Comment</button>
    </fieldset>
</form>

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

}
?>

And the comments is then echoed on the page calling the display_comments().

        <div clasS="uk-width-1-1">
<?= display_comments(2); ?>
        </div>

(the 2 is the post ID)

As you can see there's a lot of HTML inside the function, it's not ideal. How can the code be improved to remove the HTML and maybe foreach the results?

IMHO its not that bad, except proper code indention. On the other side, you can do that in so many other ways, it mostly depends on your application and/or used framework. Just one other thing, you don't need to do <?= when calling display_comments() because you have no return value in that function, you directly write to output buffer with "echo" and "non php content" in your function body. — Brain Foo Long
– Brain Foo Long, Commented Nov 7, 2015 at 16:15

bumperbox · Accepted Answer · 2016-06-07 09:01:39Z

I would split up the function into 2 parts

fetch comments and recurse
render html for comment

To me it is easier to see the looping logic without all the html in place

You could then also look at templating engine if you are going to be doing lots of this sort of thing, there are various options available, smarty, twig, mustache, blades to name a few.

function display_comments($article_id, $parent_id = 0, $level = 0)
{
    // TODO handle sql connection error
    $comments = fetch_article_comments($article_id, $parent_id);

    foreach ($comments as $comment) {
        display_comment_html($comment);

        display_comments($article_id, $comment->comment_id, $level + 1);
        echo '</div> <!-- /comment -->';
    }

}


function display_comment_html($comment)
{
    $comment_id = $comment->comment_id;
    $member_id = $comment->member_id;
    $comment_text = $comment->comment_text;
    $comment_timestamp = $comment->comment_timestamp;  //get timeAgo


// RENDER
    ?>
    <div class="comment">
    <b><?= timeAgoInWords($comment_timestamp); ?></b><br>
    <?= $comment_text ?><br>
    <a href="#" class="replyLink" data-comment-id="<?= $comment_id ?>"><span class="uk-icon-comment-o"> reply</span></a>
    <a href="recursive.php?delete=<?= $comment_id ?>"><span class="uk-icon-trash-o"> delete</span></a>


    <form action="recursive.php" method="post" class="uk-form comment_form" id="<?= $comment_id ?>">
        <fieldset data-uk-margin>
            <input type="hidden" name="id" value="<?= $article_id ?>"/>
            <input type="hidden" name="parent" value="<?= $comment_id ?>"/>
            <input type="text" placeholder="Comment" name="svar">
            <button class="uk-button">Comment</button>
        </fieldset>
    </form>
    <?php
}

Thijs Riezebeek · Accepted Answer · 2016-06-07 09:42:20Z

The way I think your website/blog is designed architecturally is where most of the improvements could be made. I suspect that you have a bunch of .html/.php files that call a bunch of functions and render some HTML. In this scenario, your function is pretty much OK besides a lot of indentation issues.

This generally leads to code that is very hard to test and maintain. I would suggest you use at least some form of MVC. This way you can use template files (View) that specify what a comment should look like in terms of HTML. Then you can render this view from your Controller after retrieving your comment Models.

user2570870 · Accepted Answer · 2015-11-09 16:57:08Z

0

what I'd do is place it on a function i.e.

foreach($data as $key){
  make_htaml($key); //render your html tags
}  

function make_html($key){
return "<a href='#'>".$key."</a>";  //values you want
}

This is some simple example, but if you have something that repeat each time, you should place it on a function and iterate over it.

answered Nov 9, 2015 at 16:57

user2570870

111 bronze badge

Add a comment |

Stack Exchange Network

Recursive comment function echoes HTML inside function

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Recursive comment function echoes HTML inside function

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions