Make WordPress Core

Opened 12 months ago

Last modified 12 months ago

#58152 assigned enhancement

Possible variable re-use

Reported by: mujuonly's profile mujuonly Owned by: mujuonly's profile mujuonly
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.3
Component: Users Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

The variable $spam is defined in the line 2228 in wp-includes/user.php and we can re-use the same variable in the line 2463 and 2646

Change History (2)

This ticket was mentioned in PR #4340 on WordPress/wordpress-develop by @mujuonly.


12 months ago
#1

  • Keywords has-patch added; needs-patch removed

The variable $spam is set above in the function like $spam = empty( $userdata['spam'] ) ? 0 : (bool) $userdata['spam']; We can re-use the variable instead of checking again as the specific key in the $userdata array is not getting changed.

Trac ticket: https://core.trac.wordpress.org/ticket/58152

#2 @SergeyBiryukov
12 months ago

  • Keywords 2nd-opinion added

Thanks for the PR!

It looks like the purpose of the $spam variable here is to be included in the compact() call in line 2330.

Reading the DocBlock above the suggested change, it makes it immediately clear what $userdata is: the raw array of data passed to wp_insert_user(), and $userdata['spam'] is a part of that array. This naming also seems more consistent when being compared with $old_user_data->spam.

If we replace it with $spam, anyone reviewing the code would then have to spend some more time to find where that variable is defined.

So I don't see any benefits in changing this code, it seems fine as is to me.

Last edited 12 months ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.