Opened 8 years ago
Closed 2 years ago
#40420 closed defect (bug) (fixed)
key/group naming confusion in _get_non_cached_ids()
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | 3.9 |
Component: | Cache API | Keywords: | has-patch |
Focuses: | performance, coding-standards | Cc: |
Description
The _get_non_cached_ids()
function uses a parameter named $cache_key
that actually represents a cache group everywhere else within WordPress.
This means when looking at the block or inline documentations, a developer may accidentally be guided down the path of passing in an actual cache-key instead of the cache-group they're checking against.
My recommendation would be to clarify this bit of code to use $group
or $cache_group
rather than $cache_key
, and update any surrounding code documentation to be explicit if it needs it. It's a non-functional change, but will make the code more clear for people.
(Related: BuddyPress comes packaged with it's own similar helper function, and opted to use $cache_group
.)
Attachments (1)
Change History (5)
#2
@
2 years ago
- Focuses coding-standards added
- Keywords has-patch added
@SergeyBiryukov Pinging as this is related to renaming variables.
40420.patch paints the bikeshed in the following ways:
$cache_group
instead of$cache_key
Notes:
$group
instead of$cache_group
everywhere else but 1 place (the customizer) which was newly introduced in 4.7.0. (See:WP_Customize_Manager->find_changeset_post_id()
)$cache_group
everywhere, but that's a lot of code-churn$group
reads too ambiguously to me, especially given the context of BuddyPress GroupsAdditional findings:
!wp_cache_get()
here, which might actually be a logic error. It's plausible this should be a strictfalse
comparison, as the intended functionality is to identify totally uncached keys, and not a value that might be an empty string or array. (This type of problem exists through-out WordPress already though.)