Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#53306 closed enhancement (fixed)

Cleaner Code admin-footer.php ln 109

Reported by: jamil95's profile jamil95 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: minor Version: 5.8
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 (7)

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


3 years ago
#1

  • Keywords has-patch added

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

Trac ticket:

#2 @SergeyBiryukov
3 years 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
3 years ago

Agree with @SergeyBiryukov

#4 @SergeyBiryukov
3 years 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
3 years 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!

#7 @desrosj
3 years ago

  • Milestone changed from Awaiting Review to 5.8
Note: See TracTickets for help on using tickets.