7
\$\begingroup\$

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?

\$\endgroup\$
7
\$\begingroup\$

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.

\$\endgroup\$
  • \$\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 Mar 28 '13 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 Mar 28 '13 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 Mar 28 '13 at 23:59
  • \$\begingroup\$ @Jon - Does my update satisfy your original query? \$\endgroup\$ – David Harkness Mar 29 '13 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 Apr 1 '13 at 8:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.