Need review on concatenation [closed]

Posted on

Problem

I am trying to concatenate the echo statements but I am failing. Also any other tips are appreciated in your review.

<?php
// load all 'category' terms for the post
$terms = get_the_terms($post->ID, 'category');

// we will use the first term to load ACF data from
if( !empty($terms) )
{
    $term = array_pop($terms);

    $custom_field = get_field('category_image', 'category_' . $term->term_id );

    // do something with $custom_field
    echo "<a href=" get_field('category_link', $custom_field) ">";
    echo    "<img src="<?php echo $image; ?>" alt="<?php echo get_cat_name($cat->term_id); ?>" class="item-image">";
    echo "</a>";
}
?>

Solution

The places where I have worked, as well as a number of articles I have read, have taught me that echoing HTML is bad practice, and really if you think about it from a maintenance perspective you want that clear separation of PHP and HTML. An example of your code with that separation:

<?php
    $terms = get_the_terms($post->ID, 'category');

    if(!empty($terms)){
        $term = array_pop($terms);
        $custom_field = get_field('category_image', 'category_' . $term->term_id );
?>
    <a href="<?php echo get_field('category_link', $custom_field); ?>">
        <img src="<?php echo $image; ?>" alt="<?php echo get_cat_name($cat->term_id); ?>" class="item-image" />
    </a>
<?php } ?>

Or if you wanted to simplify it a bit for future readers:

<?php
    $terms = get_the_terms($post->ID, 'category');

    if(!empty($terms)){
        $term = array_pop($terms);
        $custom_field = get_field('category_image', 'category_' . $term->term_id );
        $href = get_field('category_link', $custom_field);
        $cat_name = get_cat_name($cat->term_id);
?>
    <a href="<?php echo $href; ?>">
        <img src="<?php echo $image; ?>" alt="<?php echo $cat_name; ?>" class="item-image" />
    </a>
<?php } ?>

Or you could build an array out of the values if you wanted to “encapsulate” the link or something instead. Either way, a setup like this is much clearer and more maintainable than if you were to concatenate everything into one giant string and echo it.

Hope this helps.

Leave a Reply

Your email address will not be published. Required fields are marked *