I have a class with a method that will be called multiple times over an object's lifetime to perform some processing steps. This method operates on a mixture of immutable (does not change over the lifetime of the process) data and data that is passed as an argument. The immutable data is comparatively expensive to calculate, so I would like to cache it.
class Sample
{
public function process($data)
{
$immutable = $this->getImmutable();
$this->processImplementation($immutable, $data); // not interesting
}
}
How should getImmutable
be implemented?
Option #1 would be
public function getImmutable()
{
static $cache;
if ($cache === null) {
$cache = "not interesting";
}
return $cache;
}
Option #2 would be
private $_cache;
public function getImmutable()
{
if ($this->_cache === null) {
$this->_cache = "not interesting";
}
return $this->_cache;
}
Option #2 is of course better OOP, but what I like about option #1 is that the "implementation detail" $cache
is physically close to the only place where it is used. This means that it doesn't increase the mental load of someone reading the code unless that someone decides to read the body of getImmutable
, in which case the implementation detail has become important.
In my mind, this purely practical argument is stronger than theoretical purisms.
I am also aware that the static
version shares the cache across all instances of the class, which option #2 does not (and that's a good thing). However this is not an issue because
- no more than one instance of the class will be created per process
- PHP is not multithreaded so the shared cache will not be a problem even when unit testing
Is there some other argument for option #2 that could tip the scales?
1 Answer 1
I'd go with Option #3: extract $cache
and getImmutable
to a new class whose sole responsibility is providing the expensive data. Whether it caches it and how should be up to the new class. This new class will look just like Option #2 but be separated from the existing class that needs the data.
This provides encapsulation and increases testability.
Update
The ideal design that allows maximal flexibility must always be weighed against the current needs. If the former may give you 4 degrees of awesomeness but you only need 1 in the current application, you are probably better off going with a design that satisfies the latter until you need those extra features.
Encapsulation doesn't need to be applied at the class level in every instance. You can apply it at the method level until you need something more complex.
What would be more complex? One controller needs the data as a PHP model while another needs the JSON, but both may be required multiple times. In this case you'll get gains by caching each form separately. Now you'll want to separate the code to produce and transform the various data representations where the JSON producer can reuse the data-model producer.
By making the JSON controller depend on the JSON producer alone, that producer is free to reuse the data-model producer, hit the database directly, or use test data provided in a fixture for unit tests. Now you're ready to separate all these concerns.
Until then, however, a static variable hidden inside a controller method is sufficient for your needs while providing a basic level of encapsulation.
In other words, go with Option #1 and call it a day.
-
\$\begingroup\$ First of all thanks for you input. Unfortunately I don't feel that #3 is a good approach in this particular case because calculating the immutable data is 5 LOC; writing a new class for that feels like bringing in the worst parts of Java. In addition,
Sample
is in reality a controller and$data
is a collection of models; the final result goes into a view. Inserting a new actor in the play is not something I would prefer to do. I should perhaps have mentioned all this because in the general case your recommendation is good, and for that you have my +1. \$\endgroup\$Jon– Jon2013年03月28日 23:06:55 +00:00Commented Mar 28, 2013 at 23:06 -
\$\begingroup\$ In that case, given that
$cache
is used in that single method, I would encapsulate it as a static local variable (#1) in the same spirit as #3. \$\endgroup\$David Harkness– David Harkness2013年03月28日 23:37:31 +00:00Commented Mar 28, 2013 at 23:37 -
\$\begingroup\$ Yes,
$cache
is never going to be used from anywhere else. One specific controller method transforms models into one particular JSON representation (rendered by the view); the immutable data is a blueprint of what goes into the JSON, which I calculate on the fly by poking the model to see what it has to offer. \$\endgroup\$Jon– Jon2013年03月28日 23:59:14 +00:00Commented Mar 28, 2013 at 23:59 -
\$\begingroup\$ @Jon - Does my update satisfy your original query? \$\endgroup\$David Harkness– David Harkness2013年03月29日 03:43:58 +00:00Commented Mar 29, 2013 at 3:43
-
\$\begingroup\$ In the end I believe it would be best for a data-agnostic caching component to be utilized here, but since one does not exist ATM I went with localized caching as above. Thanks for your time! \$\endgroup\$Jon– Jon2013年04月01日 08:16:15 +00:00Commented Apr 1, 2013 at 8:16