Opened 9 years ago
Closed 3 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
@
3 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_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.)