Opened 9 years ago
Closed 3 years ago
#40420 closed defect (bug) (fixed)
key/group naming confusion in _get_non_cached_ids()
| Reported by: | johnjamesjacoby | Owned by: | SergeyBiryukov |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.3 |
| Component: | Cache API | Version: | 3.9 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: | performance, coding-standards |
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.)
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
40420.patch paints the bikeshed in the following ways:
$cache_groupinstead of$cache_keyNotes:
$groupinstead of$cache_groupeverywhere else but 1 place (the customizer) which was newly introduced in 4.7.0. (See:WP_Customize_Manager->find_changeset_post_id())$cache_groupeverywhere, but that's a lot of code-churn$groupreads 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 strictfalsecomparison, 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.)