Add initial functionality #1
Conversation
| /** | ||
| * Create or update the database table. | ||
| */ | ||
| private function upsert(): void { |
There was a problem hiding this comment.
Any value in returning bool here, true if it was already ready or if the db delta completed, false if not? You could check that result before declaring the featured ready.
| * @param mixed $value Item value. | ||
| * @param string $group Item group. | ||
| */ | ||
| public function set( string $key, mixed $value, string $group ): void { |
There was a problem hiding this comment.
Might it be beneficial to return bool on the set and delete methods, to mirror the corresponding object cache methods?
There was a problem hiding this comment.
this got me thinking that maybe this should do more than mirror the object cache! expanded on that in another comment
There was a problem hiding this comment.
I've recently become somewhat skeptical of these return values. In addition to the possibility of ambiguity (does false mean the value wasn't deleted or that there was nothing to delete? would you return true even if no deletions took place?), have you found a use for them?
|
Wild idea: what if this were fully compatible with the WordPress object cache's (faux) interface? Then it could be an option for a database-driven object cache. If it also used DI to take in an instance of WPDB instead of using the global |
Although I see the harmony, my initial reaction is that I'm not greatly attracted to this idea, for a couple reasons. Compatibility with the object cache "interface" requires a decent amount of additional code, some of which might be used even when The Pit wasn't being used as the object cache (TTLs), but perhaps more of which would be dead when it wasn't being used as the object cache (hit rates, compatibility with Additionally, I think it would compel the table itself to be a global table for multisite compatibility, and keys would need to be made site-specific. Again, that's complexity that runs up against (what I had in mind as) the intended scope of the library and the table. If there's interest in a database-driven object cache, might I humbly suggest looking at the Training Object Cache that I worked on a few years ago as a starting point, where a lot of that legwork has been done. See also the next PR, which opens a possibility for using The Pit as a cache store. |
Co-authored-by: Matthew Boynes <[email protected]>
|
My plan is to merge this and tag an initial release as there's a project waiting to use it. I'll keep it at major version 0 so that this discussion can continue in issues (including about the name!) and we can refine the API before tagging a 1.0 release. |
No description provided.