We are all used to WordPress magically deciding which post is the current one, and we have all written functions like follows. But I will show you why it might be a bad idea.

/**
 * Does something related to a post.
 *
 * @param int|WP_Post|null $post_id A WordPress post ID or WP_Post Object, defaults to the current post.
 * @return string
 */
function my_get_something_post_related( ?int|WP_Post $post_id ): string {
  if ( $post_id instance_of WP_Post ) {
    $post_id = $post_id->ID;
  } elseif ( $post_id === null ) {
    $post_id = get_the_ID();
  }
  
  // some code
  
  return $some_string;
}

WordPress is full of functions like it. While it is convenient and super easy to use it’s still bad. Just a few years ago I was all onboard the “yeah, WordPress does it, so will I” train.

Why is it bad?

Well, there are two main reasons why it’s a bad idea. If you’re just in the WordPress ecosystem this is common practice and you will thing it’s good practice, but my former colleague Joshua showed me an outside perspective to it.

Testability / Code complexity

The complexity to test code like this increases, you need to mock get_the_ID() and always keep this in mind. But you might throw in “Hey Christoph, I don’t write unit tests”, and that’s OK, I don’t write unit tests for every line of code either, but I try to write new code with a mindset that allows to add unit tests without too much trouble. That mindset will reduce complexity in functions which, will make errors easier to catch, even without Unit Tests.

In the example, you should consider to cover at least the following cases in a Unit test

  • null passed with get_the_ID() returning a post_id
  • null passed with get_the_ID() returning no post_id
  • WP_Post passed with the desired post type (maybe a post, but no attachment)
  • WP_Post passed with a wrong post type
  • int passed as valid post_id
  • int passed as some negative integer or zero
  • int passed as any positive integer, but there is no valid post for that ID.

If I change the code to this, it does get better.

/**
 * Does something related to a post.
 *
 * @param int|WP_Post $post_id A WordPress post ID or WP_Post Object.
 * @return string
 */
function my_get_something_post_related( int|WP_Post $post_id ): string {
  if ( $post_id instance_of WP_Post ) {
    $post_id = $post_id->ID;
  }
  
  // some code
  
  return $some_string;
}
  • WP_Post passed with the desired post type (maybe a post, but no attachment)
  • WP_Post passed with a wrong post type
  • int passed as valid post_id
  • int passed as some negative integer or zero
  • int passed as any positive integer that is not a post_id

Or we change it to this?

/**
 * Does something related to a post.
 *
 * @param int $post_id A WordPress post ID.
 * @return string
 */
function my_get_something_post_related( int $post_id ): string {
  // some code
  
  return $some_string;
}
  • int passed as valid post_id
  • int passed as some negative integer or zero
  • int passed as any positive integer that is not a post_id

And the best approach would be this.

/**
 * Does something related to a post.
 *
 * @param WP_Post $post A WordPress Post Object.
 * @return string
 */
function my_get_something_post_related( WP_Post $post ): string {
  
  // some code
  
  return $some_string;
}
  • WP_Post passed with the desired post type (maybe a post, but no attachment)
  • WP_Post passed with a wrong post type

As you see, the more precise your parameter becomes, the easier your code becomes and the more robust it will become.

Oh, and did I mention, since you will likely have 5, 10 or more functions, you reduce the complexity for all of them that.

Robustness / Predictability

If the unit tests did not yet convince you, I have more. Lets create an example, I admit this is constructed and bad code, but it will prove the point.

while ( have_posts() ) {
  the_post();
  my_magic_function_1(); // This will work as intended
  
  // We want to query some related posts here.
  $args = []; // Some args for the sub query.
  $sub_query = new WP_Query( $args );
  while ( $sub_query->have_posts() ) {
    $sub_query->the_post();
    // do something with this
  }
  
  // At this point you need to call wp_reset_postdata(), otherwise the context will be wrong.
  
  // This will reference the last post from the sub query, since I forgot to reset by design.
  my_magic_function_2();   
}

I know that the code example is broken, but the point is to show the downside of the automatic approach. And if the example is less obvious, finding the bug and debugging this, is very cumbersome. I have been in this situation, and it took way too long to debug this.

By relying less on these magic/guessing functions, the code becomes more robust, even if you do an error.

while ( have_posts() ) {
  the_post();
  $current_post = get_post(); // the only place where we will use "magic"
  
  my_magic_function_1( $current_post );
  
  // We want to query some related posts here.
  $args = []; // Some args for the sub query.
  $sub_query = new WP_Query( $args );
  while ( $sub_query->have_posts() ) {
    $sub_query->the_post();
    // do something with this
  }
  // At this point you need to call wp_reset_postdata(), otherwise the context will be wrong.
  
  my_magic_function_2( $current_post );   
}

The code example is still broken, and should not pass through a code review, but at least you won’t have any side effects and your frontend will behave like it should.

You will still be able to use this the wrong way, but it’s way easier to catch errors, either since PHP will throw them or it’s easier to catch them in code reviews.

Conclusion

I agree that you still won’t write Unit Tests right away, but adopting this will improve the code quality and will make your code less cluttered. True, you’ll need to add a very little extra thought when using the frontend. But it’s worth it and as good practice you could start using WordPress core functions alike as well get_the_title( $current_post ) will improve the intention and will actually have a minor positive impact on performance.

What are your thought on this?

Recently when reviewing code I stumbled upon $var == "some string" in code, and that sparked the discussion wether you should have strict comparison even for strings.

<?php

// Don't spoil your self, have a guess what weird idea I had to achieve this.
// The solution start at line 42.
//
// Idea by: Christoph Daum
// https://christoph-daum.de

$value = get_customer();

echo 'Loose comparison is: ';
if ( $value == 'Hans Sampleman' ) {
	echo 'True' . PHP_EOL;
} else {
	echo 'False' . PHP_EOL;
}

echo 'Strict comparison is: ';
if ( $value === 'Hans Sampleman' ) {
	echo 'True' . PHP_EOL;
} else {
	echo 'False' . PHP_EOL;
}

echo 'Explicit comparison is: ';
if ( (string) $value === 'Hans Sampleman' ) {
	echo 'True' . PHP_EOL;
} else {
	echo 'False' . PHP_EOL;
}

/*
The code will result in:

Loose comparison is: True
Strict comparison is: False
Explicit comparison is: True
*/

I’ve created proof of concept to show that it actually might be beneficial to be strict, even for strings. While this PoC is crafted and very unlikely to happen in reality, it proofs that you want to be strict.

The magic happens in get_customer() which is missing in the excerpt by design. Feel free to take a quiz and try to guess what happens inside. Feel free to comment if you figured it out without spoiling it in the comments.

This is the complete code example.