Closing php loop correctly

I am using PHP loop to output the list of post links grouped by year.

<?php
/**
 * Template for displaying archive page
 **/

get_header();
?>

    <main id="primary" class="page-archive">


  <h1>Archive</h1>

  <?php 
// https://wordpress.stackexchange.com/questions/144570/output-yearly-archive-within-a-page
$posts = new WP_Query (
  array (
    'post_type'=>'post', 
    'post_status'=>'publish', 
    'posts_per_page'=>-1
    )
  ); 
 
if ( $posts->have_posts() ) : ?>
 

 
    <!-- the loop -->
    <?php while ( $posts->have_posts() ) : $posts->the_post(); 
          $year = get_the_time('Y');

           if ($posts->current_post === 0) 

           printf( '<h3>%s</h3>', $year ); 
           
          elseif ($last_year !== $year)
           printf( '<h3>%s</h3>', $year );
      

    ?>       
          <div>
                
                <a href="<?php the_permalink() ?>"><?php the_title() ?></a>
                <div><?php the_time( 'j F Y' ) ?></div>
            </div>


    <?php
            if ( ( $posts->current_post + 1 ) === $posts->post_count ) echo '</li>'; // Always close the <li> at the end of the loop
        $last_year = $year;

        endwhile;

    ?>


    <?php endif ?>
    <!-- end of the loop -->
 

    </main><!-- #main -->

<?php
get_sidebar();
get_footer();

In the original code conditional if statement is used to close li

 <?php
            if ( ( $posts->current_post + 1 ) === $posts->post_count ) echo '</li>'; // Always close the <li> at the end of the loop
        $last_year = $year;

        endwhile;

    ?>

Since I am not using HTML list to output posts and don’t need ul and li, what would be the correct way to close the above loop when theres no more posts and without using echo ‘li’; ?

Simply delete the line.

The loop is ‘closed’ by the endwhile statement; it has nothing to do with the line in question. If all of your output blocks are self-contained and have no enclosing element, there’s no need for that line at all. For that matter, they could have simply stuck the </li> outside the loop and not forced the code to evaluate the if condition on each iteration. Wasteful.

$x = 0;
echo "Before Loop HTML <li>";
while ($x < 5) {
  echo $x
  x++;
}
echo "</li>After Loop HTML";
1 Like

Thank you for the quick reply @m_hutley. As you suggested I deleted the following line from the above code:

if ( ( $posts->current_post + 1 ) === $posts->post_count ) echo '</li>';

Now the code is

 <?php
            
        $last_year = $year;

        endwhile;

    ?>

and it works as expected.

I find it unnecessarily complicated jumping in and out of PHP in order to display HTML script also far prefer to have consistent block delimiters.

Is the amended script any easier?

<?php declare(strict_types=1);
// following two lines should be set in php.ini
error_reporting(-1); // maximu error reporting
ini_set('display_errors', 'true'); // only show errors locally

/**
 * Template for displaying archive page
 **/
get_header();

// https://wordpress.stackexchange.com/questions/144570/output-yearly-archive-within-a-page
$posts = new WP_Query 
(
  array (
    'post_type'     => 'post', 
    'post_status'   => 'publish', 
    'posts_per_page'=> -1
    )
  ); 

echo '<main id="primary" class="page-archive">';
  echo '<h1>Archive</h1>';

  if ( $posts->have_posts() ) : 
    // <!-- the loop -->
    while ( $posts->have_posts() ) : 
      $posts->the_post(); 
      $year = get_the_time('Y');

       if ($posts->current_post === 0) :
          printf( '<h3>%s</h3>', $year ); 

       elseif ($last_year !== $year) :
          printf( '<h3>%s</h3>', $year );
  
          echo '<div>';
            echo '<a href="' .the_permalink() .the_title() .'</a>';
            echo '<div>' .the_time( 'j F Y' ) .'</div>';
          echo '</div>';

          // Always close the <li> at the end of the loop
          if ( ( $posts->current_post + 1 ) === $posts->post_count ) :
            echo '</li>'; 
          endif;  
          $last_year = $year;
        endif;
    endwhile; // <!-- end of the loop -->
  endif; 

echo '</main><!-- #main -->';

get_sidebar();
get_footer();

Thanks for further feedback on this @John_Betong. I don’t know much PHP. I just googled the code and then further tried to google what each line of code is doing in that WordPress loop. But definitely your version of code is a good learning opportunity for me.

You mentioned :

I currently have my WordPress install on Laragon on my Windows machine. I found php.ini in Laragon menu. Should I just try add above two lines there?

I also use VS Code to adjust WordPress templates when converting HTML/CSS markup into WP. I didn’t install any PHP specifically on my PC. My understanding it was automatically installed with Laragon…

1 Like

I would suggest that you stop opening and closing PHP willy-nilly, stop using the cryptic and hard to follow VB style PHP nomenclature in favor of the C style one, stop performing checks you don’t actually need, and remember that if there are posts you’ll be open so just output the close, instead of assuming you need to check for it!

<?php

get_header();

echo '
			<main id="primary" class="page-archive">
				<h2>Archive</h2>';

$posts = new WP_Query([
	'post_type'      => 'post',
	'post_status'    => 'publish',
	'posts_per_page' => -1
]);

if ($posts->have_posts()) {

	$last_year = false;

	do {

		$posts->the_post();

		$year = get_the_time('Y');
		if (($last_year != $year) {

			if ($last_year) echo '
				</section>';

			$last_year = $year;

			echo '
				<section>
					<h3>', $year, '</h3>';

		}

		echo '
					<div>
						<a href="', php the_permalink(), '">', the_title(), '</a>
						<time>', the_time( 'j F Y' ), '</time>
					</div>';

	} while ($posts->have_posts());

	echo '
				</section>';

}

echo '
			</main>';

get_sidebar();
get_footer();

See how the false at the start makes it skip the internal /section on the first loop. Final loop you assume we looped and just output it?

Also the switch to do/while skips an unnecessary comparison. If you just “if” there’s no reason for normal while() apart from blindly copying what WP’s developers put in their docs. That’s not on you, that’s on them.

That said, from a structural / semantic standpoint, those DIV really should be LI inside a UL. Not sure why you’re swapping that out for the tags devoid of semantic meaning.

1 Like

Thanks for your feedback Jason

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.