#53306 closed enhancement (fixed)
Cleaner Code admin-footer.php ln 109
Reported by: | jamil95 | Owned by: | 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
#2
@
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 :)
#4
@
3 years ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 51053:
SergeyBiryukov commented on PR #1318:
3 years ago
#5
Thanks for the PR! Merged in https://core.trac.wordpress.org/changeset/51053.
Merged if statement for cleaner code in wp-admin/admin-footer.php
Trac ticket: