WordPress.org

Make WordPress Core

Opened 2 weeks ago

Closed 2 weeks ago

Last modified 12 days ago

#53306 closed enhancement (fixed)

Cleaner Code admin-footer.php ln 109

Reported by: jamil95 Owned by: SergeyBiryukov
Milestone: Awaiting Review Priority: normal
Severity: minor Version: trunk
Component: Administration Keywords: has-patch
Focuses: coding-standards Cc:

Description

Before:

<?php
if ( function_exists( 'get_site_option' ) ) {
        if ( false === get_site_option( 'can_compress_scripts' ) ) {
                compression_test();
        }
}

After:

<?php
if (function_exists('get_site_option') && false === get_site_option('can_compress_scripts')) {
    compression_test();
}
?>

Change History (6)

This ticket was mentioned in PR #1318 on WordPress/wordpress-develop by jamilessifi.


2 weeks ago

  • Keywords has-patch added

Merged if statement for cleaner code in wp-admin/admin-footer.php

Trac ticket:

#2 @SergeyBiryukov
2 weeks ago

  • Component changed from General to Administration

Hi there, welcome to WordPress Trac! Thanks for the ticket and the PR.

The existing code seems fine as is to me, it makes it clear that the comment only applies to the function_exists() check and not to the whole conditional.

Personally, I find separate conditions on their own lines (where appropriate) easier to read than a longer line with multiple conditions.

It also looks like the proposed code does not pass the WordPress Coding Standards check:
https://github.com/WordPress/wordpress-develop/runs/2709582165?check_suite_focus=true

Another option would be something like this, which is consistent with the pattern suggested in the "Complex control structure conditions" example in Updating the Coding standards for modern PHP proposal, and used since then in many other places in core:

// get_site_option() won't exist when auto upgrading from <= 2.7.
if ( function_exists( 'get_site_option' )
	&& false === get_site_option( 'can_compress_scripts' )
) {
	compression_test();
}

I'm not sure it offers any significant improvement over the current code either, but if the goal is to use a single if condition here, this would be my preference :)

#3 @mukesh27
2 weeks ago

Agree with @SergeyBiryukov

#4 @SergeyBiryukov
2 weeks ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 51053:

Coding Standards: Simplify a condition in wp-admin/admin-footer.php.

Props jamil95, mukesh27.
Fixes #53306.

#6 @jamil95
12 days ago

Sorry for the late response and thanks a lot for the feedback. I am new to PHP so this means a lot to me!

Note: See TracTickets for help on using tickets.