PHP: constants vs private static variables and understanding the intent of code

G'day:
I was looking at some code the other day, and got into a conversation with the author about it. I can't show you the actual code (and, indeed, I'll be fictionalising the content of the conversation for editorial reasons), but here's a contrived reproduction of the situation.

There is a function:

function someFunction($arg1, $arg2) {
    $intermediaryValue = $this->firstDependency->prepare($arg1);
    return $this->secondDependency->process($intermediaryValue, $arg2);
}

Don't worry about that, what I was looking at were the tests for the function. Those dependencies needed to be mocked-out, and for the purposes of this function, it's important that what's returned from someFunction is exactly the result of the process method. So we need to specifically test for that.

class SomeClassTest extends \PHPUnit_Framework_TestCase {

    const MOCKED_PREPARE_RESULT = 'MOCKED_PREPARE_RESULT';
    const MOCKED_PROCESS_RESULT = 'MOCKED_PROCESS_RESULT';

    private $mockedFirstDependency;
    private $mockedSecondDependency;
    private $someObject;
    
    
    function setup(){
        $this->mockedFirstDependency = $this->getMockedFirstDepenedency();
        $this->mockedSecondDependency = $this->getMockedSecondDepenedency();
        $this->someObject = new SomeClass($mockedDependency);
    }
    
    /**
    * @dataProvider provideCasesForSomeFunctionTests
    */
    function testSomeFunction($testArg1, $testArg2){
        $this->mockedFirstDependency->method('prepare');
            ->with($testArg1, $testArg2)
            ->willReturn(self::MOCKED_PREPARE_RESULT);
            
        $result = $this->someObject->someFunction($testArg1, $testArg2);
        
        $this->assertEquals(self::MOCKED_PROCESS_RESULT, $result);
        
    }
    
    function provideCasesForSomeFunctionTests(){
        return [
            'first variation' => ['testArg1'=>'test value 1', 'testArg2'=>'test value 2'],
            'second variation' => ['testArg1'=>'test value 3', 'testArg2'=>'test value 4']
        ];
    }
    
    private function getMockedFirstDependency(){
        $mockedFirstDependency = $this->getMockBuilder('\some\namespace\FirstDependency')
            ->setMethods(['prepare'])
            ->getMock();
            
        return $mockedFirstDependency;
    }
    
    private function getMockedSecondDependency(){
        $mockedSecondDependency = $this->getMockBuilder('\some\namespace\SecondDependency')
            ->setMethods(['process'])
            ->getMock();
        
        $mockedSecondDependency->method('process')
            ->with(self::MOCKED_PREPARE_RESULT)
            ->willReturn(self::MOCKED_PROCESS_RESULT);
            
        return $mockedSecondDependency;
    }

}

What I was specifically looking at was the usage of constants here. Chiefly I was wondering "why are those constants?" I scanned the rest of the code and they were only ever used in this class (and remember it's a test class), and the names and the values are very specific to their usage in said class.

So I asked. The reason why they're constants is because they're unchanging values. Hmmm. Well... that doesn't wash, because a variable that one simply doesn't change also doesn't change value. So that's not valid. There is more to that necessary to determine something ought to be a constant.

Firstly, in PHP - until PHP7 anyhow - constants are intrinsically public. And according to the principle of encapsulation and the notion of information hiding, the only thing in a class that ought to be public is stuff that is specifically supposed to be public. IE: the API to the class. By inference, if you make something public, it's a flag saying "use this publicly... it's there specifically for you to do that". That is not valid here, so it's invalid for the value to be public, so - even if the value/usage was appropriate to be a constant - it kinda can't be one.

Secondly... the usage here simply isn't that of a constant. The rationalisation that its value is a constant/static/literal value therefore it should be a constant is specious. That's not how it works. Let's have a look at how we ended up with this constant being created.

Firstly we had this pre-refactoring version of the code:

function testSomeFunction($testArg1, $testArg2){
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn('MOCKED_PREPARE_RESULT');
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals('MOCKED_PROCESS_RESULT', $result);
    
}

// [...]

private function getMockedSecondDependency(){
    $mockedSecondDependency = $this->getMockBuilder('\some\namespace\SecondDependency')
        ->setMethods(['process'])
        ->getMock();
    
    $mockedSecondDependency->method('process')
        ->with('MOCKED_PREPARE_RESULT')
        ->willReturn('MOCKED_PROCESS_RESULT');
        
    return $mockedSecondDependency;
}

Note how we have a coupled duplicated values there: MOCKED_PREPARE_RESULT and MOCKED_PROCESS_RESULT. That's not DRY, so obviously there's some refactoring to do. Before we refactor, let's pause for a second. Imagine the code was like this:

function testSomeFunction($testArg1, $testArg2){
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn('GENERIC_DUMMY_VALUE');
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals('GENERIC_DUMMY_VALUE', $result);
    
}

We have the same duplication twice (he says, being deliberately tautological) in the same function. What do we do? We extract it to a variable:

function testSomeFunction($testArg1, $testArg2){
    $genericDummyValue = 'GENERIC_DUMMY_VALUE';
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn($genericDummyValue);
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals($genericDummyValue, $result);
    
}

Now all things being equal, that's a fine enough solution. If it was in a "real" class as opposed to just a test one might conclude that having a magic value in the middle of the code like that might be poor play, but in this case it's fine.

But in our case, the usage of the values is spread across two functions, so using function-local variables doesn't help it. What do we do when we need the same variable across more than one function? We hoist it up to be a class variable (well, two: $mockedPrepareResult and $mockedProcessResult):

private $mockedPrepareResult = 'MOCKED_PREPARE_RESULT';
private $mockedProcessResult = 'MOCKED_PROCESS_RESULT';

// [...]

function testSomeFunction($testArg1, $testArg2){
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn($this->mockedPrepareResult);
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals($this->mockedProcessResult, $result);
    
}

// [...]

private function getMockedSecondDependency(){
    $mockedSecondDependency = $this->getMockBuilder('\some\namespace\SecondDependency')
        ->setMethods(['process'])
        ->getMock();
    
    $mockedSecondDependency->method('process')
        ->with($this->mockedPrepareResult)
        ->willReturn($this->mockedProcessResult);
        
    return $mockedSecondDependency;
}

And one might go one step further and conclude these variables are indeed class variables and not instance variables, so one makes them static as well:

private static $mockedPrepareResult = 'MOCKED_PREPARE_RESULT';

// [...]

    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn(self::$mockedPrepareResult);

Bear in mind that in common English "constant" and "static" are analogous, but in programming usage they are for the most part unrelated. A constant is an immutable value. a static value is one that is specific to the class it's in, rather than an instance of that class. There's no overlap in the two notions in either of those contexts the terms have meaning.

So as to why one would not then go one step further and change them from private static variables to constants... it's because that's a complete non-sequitur. At no point is there any indication that the usage of these values here implies that they're supposed to be constants. They're just variables, and in the usage of these variable it would be a logic error to change the value between separate references to the variable. This is analogous to this code:

$theTotal = $this->getTheTotal();
$theTotal += 1;
return $theTotal;

It'd be a logic error to increment $theTotal by one, so don't bloody do it. It's not down to a language construct to prevent this, it's down to the programmer not being a div.

I doubt any developer would do this:

final $theTotal = $this->getTheTotal();
$theTotal += 1;
return $theTotal;

(pre-supposing PHP has the final keyword, which it doesn't). Thus putting it down to the compiler to catch the logic error.

What's a constant then?

Well whether or something ought to be a constant is down to two things. First the constant's value. It's not so much that "one oughtn't change it" (although preventing that is a side effect of something being a constant), it's that the value of the constant doesn't change.

This is a sensible usage of constants:

class Numbers {

    const BRACE = 2;
    const DOZEN = 12;
    const SCORE = 20;
    const GROSS = 144;
    
    // [...]
}

It's not so much that one doesn't want to change the value of brace, it's that the value of what it is to be a brace doesn't change. It's constant. That's when one would use a constant.

The second criterion as to why one might make a value a constant is that one actively wants to expose it publicly. A good real-world example of constants are in Symfony 2's Response class:

class Response
{
    const HTTP_CONTINUE = 100;
    const HTTP_SWITCHING_PROTOCOLS = 101;
    const HTTP_PROCESSING = 102;            // RFC2518
    const HTTP_OK = 200;
    const HTTP_CREATED = 201;
    const HTTP_ACCEPTED = 202;
    const HTTP_NON_AUTHORITATIVE_INFORMATION = 203;
    const HTTP_NO_CONTENT = 204;
    const HTTP_RESET_CONTENT = 205;
    // etc you get the idea

So when creating responses in our controllers, we don't do stuff like:

return Response::create($twig->render('clientError.twig'), 404);

Instead we do this:

return Response::create($twig->render('clientError.twig'), Response::HTTP_NOT_FOUND);

It just make the code more clear as to what's going on.

Also note that these example usages are all publicly exposed, and that's their intended usage. In that Numbers class if we only ever used the notion of a BRACE internally, and didn't have reason to expose it (it's a not oft-used word, after all), they we'd not make a constant, we'd just have a private static variable.

Realistically constants are something that one generally won't want to use. It's only in those rare situations where one has a literal value which has a specific constant meaning, and could benefit from being given a human-friendly (/code-clarity-friendly) label. Most values one will want to expose from a class or an object will not be simple literal values, so one would be exposing them via methods.

And it's a misunderstanding of the intended purpose of constant to conclude they're for literal values you're unlikely to want to change (in just a logical sense). They're for literal values which don't. That's a different thing.

Righto.

--
Adam