#56804 closed enhancement (fixed)
More readable and faster check for array of arrays in register_rest_route()
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | 6.1 |
Component: | REST API | Keywords: | has-patch commit |
Focuses: | performance | Cc: |
Description
<?php if ( count( array_filter( $arg_group['args'], 'is_array' ) ) !== count( $arg_group['args'] ) ) {
was used as a custom implementation of an any()
function.
This is not very readable and not that nice performance-wise (array_filter()
will always loop the full array, even though we are only interested in finding one (the first) non-array).
This should be simplified to a foreach
loop that is left early once a match is found, for better readability and higher performance, e.g.:
<?php foreach( $arg_group['args'] as $arg ) { if ( ! is_array( $arg ) ) { _doing_it_wrong( __FUNCTION__, sprintf( /* translators: 1: $args, 2: The REST API route being registered. */ __( 'REST API %1$s should be an array of arrays. Non-array value detected for %2$s.' ), '<code>$args</code>', '<code>' . $clean_namespace . '/' . trim( $route, '/' ) . '</code>' ), '6.1.0' ); break; // Leave the foreach loop once one non-array argument was found. } }
(Follow-up to #51986 where this suggestion missed the deadline for 6.1-RC1, taking into account the linked-to Slack discussion in that ticket, regarding the target milestone. The Version of this ticket should be 6.1, but that does not yet exist in Trac.)
Change History (6)
This ticket was mentioned in PR #3453 on WordPress/wordpress-develop by @SergeyBiryukov.
2 years ago
#1
- Keywords has-patch added
#2
@
2 years ago
- Keywords commit dev-feedback added
- Milestone changed from 6.1.1 to 6.1
Thanks for the ticket! The suggested code does look better to me in terms of readability and performance.
I believe this can be included in 6.1 RC2.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
2 years ago
#4
@
2 years ago
- Keywords dev-feedback removed
As per today's bug scrub: @costdev and I both reviewed the proposed PR, and it looks like it's good to go, thanks!
#5
@
2 years ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 54518:
@SergeyBiryukov commented on PR #3453:
2 years ago
#6
Merged in r54518.
Trac ticket: https://core.trac.wordpress.org/ticket/56804