HiveBrain v1.2.0
Get Started
← Back to all entries
principlephpMinor

Static variable in method vs private class property

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classmethodprivatepropertyvariablestatic

Problem

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?

Solution

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.

Context

StackExchange Code Review Q#24480, answer score: 7

Revisions (0)

No revisions yet.