-
Notifications
You must be signed in to change notification settings - Fork 195
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
Move SitesCache invocation out of Site model #299
Conversation
public function __construct() { | ||
$this->cache = Terminus::get_cache(); | ||
} | ||
|
||
// Returns the UUID of the site | ||
public function find($name, $options = array()) { | ||
public function _find($name, $options = array()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this function be private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... I wasn't really sure. The instance is mostly there for OOPness, but I don't really expect the instance to be really used. I'll just make it private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TeslaDethray I just made the method private. Can you please take another look at this PR? Thanks!
+1
That being said, it's neither elegant nor great for readability. PHP overloading is a strange game, and the only winning move is not to play. |
Move SitesCache invocation out of Site model
Make responsibilities a little clearer and the Site model a little more free-standing by removing any reliance on caching.
Question: what's the best way to shadow static methods with instance methods? Apparently PHP won't let them have the same name.