Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of static object access #7

Merged
merged 1 commit into from
Feb 6, 2018

Conversation

schlessera
Copy link
Contributor

This is just one possible way of getting rid of the static access.

  • Adds a factory to share an object cache instance. This replaces the getInstance() call.
  • Uses the global $wp_object_cache. This already exists in WordPress, and external code might expect it to be populated.

This is just one possible way of getting rid of the static access.

- Adds a factory to share an object cache instance. This replaces the `getInstance()` call.
- Uses the global `$wp_object_cache`. This already exists in WordPress, and external code might expect it to be populated.
@felixarntz
Copy link
Owner

I just noticed this PR, and had already worked on this: https://github.com/felixarntz/wp-psr-cache/blob/master/src/ObjectCacheService.php

What do you think? It's not amazing, but doesn't introduce a global variable.

@schlessera
Copy link
Contributor Author

The global variable already exists in WordPress, and other code will use it. If you don't fill that global variable with an object instance, some plugins might break.

@schlessera
Copy link
Contributor Author

@schlessera
Copy link
Contributor Author

@felixarntz
Copy link
Owner

That generally makes sense, I'll look into it further soon and how it is used.

@felixarntz felixarntz merged commit 23f0b0a into felixarntz:master Feb 6, 2018
@felixarntz
Copy link
Owner

I merged the PR, with a few tweaks:

  • The factory class is only used to create instances, not to share or store.
  • The service class which I had implemented before still exists and is used for static access to the main object cache instance.
  • The main instance is stored in the global $wp_object_cache as you suggested, for compatibility with some core edge-cases.
  • The service class simply uses that global and is an alternate access point. It is an abstraction that allows to possibly get rid of the global in the future, and the developer tweaking the object-cache.php file can simply continue to use the service class and set their object cache instance to use there, after creating it with the factory.

@schlessera schlessera deleted the avoid-static-calls branch February 6, 2018 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants