Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#19012 reviewing enhancement

get_ID and the_ID should accept $id as a parameter

Reported by: peterchester Owned by: ryan
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3
Component: Posts, Post Types Keywords: needs-testing needs-codex has-patch
Focuses: Cc:

Description (last modified by markjaquith)

I see throughout the WordPress code as well as throughout many plugins and themes that get_ID() is often accompanied by a test to see if an ID had been passed. It would be a harmless but extremely useful update to simply allow get_ID() and the_ID() to accept an optional $id and pass it through if it's numerical.

I've already gone ahead and created the patch (attached)

Attachments (1)

post-template.diff (993 bytes) - added by peterchester 3 years ago.
Updated post template diff with (int) cast instead of is_numeric

Download all attachments as: .zip

Change History (11)

comment:1 peterchester3 years ago

It's pretty straight forward. Basically, I see a TON of repeated code all over the place that basically looks like this:

function myfunction ($post_id = null) {
	if ($post_id == null) {
		$post_id = get_ID();

I'm proposing that we abstract that effort to the get_ID function itself so that one could simply do this:

function myfunction ($post_id = null) {
	$post_id = get_ID( $post_id );

It should have no effect on existing code at all since it's an optional parameter, but moving forward it could be super helpful to everyone.

comment:2 peterchester3 years ago

  • Cc peter@… added

comment:3 markjaquith3 years ago

  • Description modified (diff)
  • Keywords needs-patch 2nd-opinion added; has-patch removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner changed from peterchester to ryan
  • Status changed from new to reviewing

get_post() does something similar. It takes a post object, a post id, or nothing, and gives you a post object.

I wouldn't use is_numeric() though — too permissive.

Numeric strings consist of optional sign, any number of digits, optional decimal part and optional exponential part. Thus +0123.45e6 is a valid numeric value. Hexadecimal notation (0xFF) is allowed too but only without sign, decimal and exponential part.

Instead I'd say we cast to integer, and check for it being greater than zero. I'm also not sure it's useful for the_ID().

comment:4 peterchester3 years ago

Thanks for the review Mark!

Including updated the patch with integer cast instead of is_numeric().

	if ( (int) $id > 0 ) {

And as to the question of if this is useful, i cite as an example, 3 cases in wp-include/post-thumbnail-template.php such as:

function has_post_thumbnail( $post_id = null ) {
	$post_id = ( null === $post_id ) ? get_the_ID() : $post_id;
	return (bool) get_post_thumbnail_id( $post_id );

With this update this sort of thing can be updated to read:

function has_post_thumbnail( $post_id = null ) {
	$post_id = get_the_ID( $post_id );
	return (bool) get_post_thumbnail_id( $post_id );

Or even:

function has_post_thumbnail( $post_id = null ) {
	return (bool) get_post_thumbnail_id( get_the_ID( $post_id ) );

peterchester3 years ago

Updated post template diff with (int) cast instead of is_numeric

comment:5 scribu3 years ago

Could you provide an example where passing an id to get_ID() would be useful?

comment:6 peterchester3 years ago

Hi scribu! Actually, I just posted an example in my previous comment.

The problem is that the wordpress code (as well as just about every template and plugin that i've ever encountered) is riddled with phrases that go like this:

function my_function( $post_id = null ) {
	$post_id = ( null === $post_id ) ? get_the_ID() : $post_id;

I'm proposing that we could help this along by simply passing through the post id if it's set.

function my_function( $post_id = null ) {
	$post_id = get_the_ID( $post_id );

This way, if $post_id was not set, then it's returned contextually, if it is, then it's just passed through.

comment:7 scribu3 years ago

  • Keywords 2nd-opinion removed

Ah, sorry, didn't see it. Sure, makes sense to move the check lower down.

comment:8 scribu3 years ago

  • Keywords has-patch added; needs-patch removed

comment:9 nacin3 years ago

One PHP5 option is:

$post_id = get_post( $post_id )->ID;

That'll work with null, an ID, or even a post object being passed.

I think this is a bit clearer than get_the_ID() being able to take an ID parameter, which seems really confusing.

In many cases, we have functions that do these checks instead of just passing null etc. further down the stack, which is appropriate sometimes. One example is has_post_thumbnail(), which could simply pass null to get_post_thumbnail_id().

comment:10 nacin3 years ago

In [19029]:

Pass $post_id arg directly to get_post_thumbnail_id() from has_post_thumbnail(). The null/get_the_ID() check is already performed at that level of the stack. see #19012.

Note: See TracTickets for help on using tickets.