WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#41987 closed enhancement (worksforme)

Should use strict comparison at line 71 in wp-admin/admin.php

Reported by: rnaby Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Administration Keywords: close reporter-feedback
Focuses: Cc:

Description

Well, the both side of comparison has already been same data type, but I think it would be better if the comparison would be strict !

Before or now the code looks like this-

if ( $c <= 50 || ( $c > 50 && mt_rand( 0, (int)( $c / 50 ) ) == 1 ) ) {

After fix the code looks like this-

if ( $c <= 50 || ( $c > 50 && mt_rand( 0, (int)( $c / 50 ) ) === 1 ) ) {

I think the later approach is better.

Attachments (1)

41987.diff (762 bytes) - added by rnaby 4 years ago.
The diff file I generated after fixing by RabbitVCS.

Download all attachments as: .zip

Change History (6)

@rnaby
4 years ago

The diff file I generated after fixing by RabbitVCS.

#1 @dd32
4 years ago

  • Keywords close reporter-feedback added

I don't I see any benefit in strict type checking in scenario's like this or #41988

As mt_rand() only returns int or false and we don't really need to care about any non-int return values, especially in this section of code.

A scenario where it would be useful is cases where "falsey" values are expected, such as #21249

What benefit does the proposed changes actually give?
FWIW our PHP Coding Standards are available here: https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/

#2 @swissspidy
4 years ago

  • Version trunk deleted

Please note that there's also #41057 to fix coding standards in core once and for all. Much easier than having dozens of very small tickets for things not really worth fixing on their own.

#3 @SergeyBiryukov
4 years ago

  • Component changed from General to Administration

#4 @rnaby
4 years ago

  • Resolution set to worksforme
  • Status changed from new to closed

#5 @SergeyBiryukov
3 years ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.