HTTP class contains redundant defined() check

[TobiasBg]

Line ​554 of wp-includes/class-http.php contains a somewhat redundant logic:

if ( ! defined('WP_HTTP_BLOCK_EXTERNAL') || ( defined('WP_HTTP_BLOCK_EXTERNAL') && WP_HTTP_BLOCK_EXTERNAL == false ) )

The part after the

will only be executed if the constant is defined, thus it's overhead to check that again. Also checking constants for false (like WP_HTTP_BLOCK_EXTERNAL == false) is not good practice.

I have attached two patches, one that simply removes the redundant check and simplifies the WP_HTTP_BLOCK_EXTERNAL == false to ! WP_HTTP_BLOCK_EXTERNAL.
The second patch has the same logic, but has ​De Morgan's laws applied. I could decide what looks better :-)

[TobiasBg]

Patch to fix reduncancy and clean up comparison

[TobiasBg]

Same patch, but with De Morgan's laws applied

[nacin]

I'm pretty sure we do this redundancy in multiple places. Used to be more, though I know we've eliminated some of them.

[TobiasBg]

I grep'ed through the core source, but couldn't find any more occurances.

[TobiasBg]

There are three or four more files that have two defined() checks in one line, but those check for different constants.

[nacin]

Fair enough, looks like this is the last one then.

[nacin]

(In [13422]) Clean up redundant defined() check. Props TobiasBg fixes #12375

[najamelan]

I just wanted to share an idea on the matter. Surely checking if it's defined is necessary to avoid php spewing warnings, however I don't like either of the patches. It seems the proper way to do this is to test somewhere early on in code that is garanteed to run on every request (wp-settings.php) like this:

if( ! defined( WP_HTTP_BLOCK_EXTERNAL ) )

    define( WP_HTTP_BLOCK_EXTERNAL, false );

Now it can be counted on to exist everywhere, and code can be simplified where it matters. I mean it's still extra code to work around the lack of an elegant matter in php to express this, but the test exists only once, far away from code logic.

It might seem strange when there is only one occurrence of this elsewhere, but that's right now, and might change later since there is quite some inconsistencies in the user interface when using WP_HTTP_BLOCK_EXTERNAL. It will throw errors on the plugins page because it can't connect to wordpress, it will still show you update as quick action even though this can no longer work, it will want to put a plugin count in the menu bar, on the admin-bar etc...

On top of that this could be done with all defines used in the core to clean things up all over.

[TobiasBg]

This ticket was closed on a completed milestone. Please open a new ticket for your suggestion.
Note that the handling of constants has already changed a lot since this ticket was closed.