Php – Is declaring fields on classes actually harmful in PHP

PHP

Consider the following code, in which the setter is deliberately broken due to a mundane programming error that I have made for real a few times in the past:

<?php

    class TestClass {

        private $testField;

        function setField($newVal) {
            $testField = $newVal;
            // deliberately broken; should be `$this->testField = $newVal`
        }

        function getField() {
            return $this->testField;
        }

    }

    $testInstance = new TestClass();
    $testInstance->setField("Hello world!");

    // Actually prints nothing; getField() returns null
    echo $testInstance->getField(); 

?>

The fact that I declared $testField at the top of the class helps conceal that programming error from me. If I hadn't declared the field, then I would get something similar to the following warning printed to my error log upon calling this script, which would potentially be valuable to helping my debugging – especially if I were to make an error like this in a large and complicated real-world application:

PHP Notice: Undefined property: TestClass::$testField in /var/www/test.php on line 13

With the declaration, there is no warning.

Perhaps I'm missing something, but I'm aware of only two reasons to declare class fields in PHP: firstly, that the declarations act as documentation, and secondly, that without declarations one can't use the private and protected access modifiers, which are arguably useful. Since the latter argument doesn't apply to public fields – assigning to an undeclared field of an object makes it public – it seems to me that I ought to at least comment out all my public field declarations. The comments will provide the exact same documentation value, but I will benefit from warnings if I try to read an uninitialized field.

On further thought, though, it doesn't seem to make sense to stop there. Since in my experience trying to read an uninitialized field is a much more common cause of error than trying to inappropriately read or modify a private or protected field (I've done the former several times already in my short programming career, but never the latter), it looks to me like commenting out all field declarations – not just public ones – would be best practice.

What makes me hesitate is that I've never seen anybody else do it in their code. Why not? Is there a benefit to declaring class fields that I'm not aware of? Or can I modify PHP's configuration in some way to change the behavior of field declarations so that I can use real field declarations and still benefit from "Undefined property" warnings? Or is there anything else at all that I've missed in my analysis?

Best Answer

You should always declare your class properties ahead of time. While PHP is a dynamic language and will happily go right along with you creating your properties at runtime, there are several downsides to this path.

The compiler can optimize declared properties. When you dynamically declare a property this is a performance hit as the compiler has to create a dynamic hash table to store your dynamic properties.
I'm not 100% on this one but I believe that bytecode optimizers like APC won't be as useful since they won't have a full picture of your class. (And optimizers like APC are a must)
Makes your code 1000x harder to read. People will hate you.
Makes it 1000x more likely that you will make a mistake. (Was it getId or getID? Wait, you used both. Fail.)
No IDE autocompleting or type hinting.
Documentation generators won't see your property.

The problem you described is actually a minor one when compared to the problem of not declaring your properties in your class definition. Here's a good solution.

Get used to declaring defaults for your properties.

private $testField = null;
private $testField = '';
private $testField = 0;
private $testField = []; //array()
private $testField = false;

Except for properties that will be storing objects, this will cover the majority of base types that you will be storing. The properties that will be storing objects can be set in your constructor.

A good rule for class design is that after your object has been created and your constructor has run you shouldn't have any properties out there "undefined".

Related Solutions

Php – way to document required properties in traits (PHP)

The best way I have found to do this is as follows:

class This_Is_My_Class {
    use My_Trait;

    protected $prefix = '';

    function __construct() {
        $this->prefix = 'not_empty';
    }

    protected function get_prefix() {
       if ( empty($this->prefix) ) {
           throw new Exception("Prefix must be non empty.");
       }
       return $this->prefix;
    }

    ... rest of class here
}

trait My_Trait {

    abstract protected function get_prefix();

    protected function do_whatever() {
        $prefix = $this->get_prefix();
        ... do stuff
    }
}

Now, when using the trait, you properly get an exception when attempting to do things with it when the class using it either lacks the get_prefix function (due to the abstract declaration) or tries to use the prefix (due to the exception in the parent class).

Of course, if there is a cleaner solution to enforce that a class using a trait has a particular property, I'd love to hear it.

Php – How to pass a mock object into a class for unit tests

However, I'm not using Dependency Injection so I cannot use this approach.

Are you sure you're not? Because when you write code like this:

class Example
{
    private $dependency;

    public function __construct(Dependency $dependency)
    {
        $this->dependency = $dependency;
    }
}

That screams dependency injection at me (your first Example above is ready for constructor injection). After all dependency injection is nothing more then a fancy term for passing a parameter in to become part of state. Some like to use DI container frameworks or libraries to do their DI in what amounts to a new language but others like to stick with the tools built into their language and use Pure DI. What you provided above is an example of Pure DI.

Either way you do it the main point is to separate construction code from behavior code. So long as you do that you get DI's main benefit: the ability to reconfigure without much rewriting.

A simple pattern for using Pure DI is to do the composing (constructing) in main (or as far up the call stack as you can get). But in PHP main is just the first line of code. So assume that's where the following lives:

$example = new Example( new Dependency() );

There's your injection. No DI container needed. If you have more injections for more objects to do you put them here as well. Doing this you can build up a connected object graph. If you need to change Dependency so that it depends on something else you build that in here as well. Whether you give them $ names is up to you. Whatever is most readable. Just don't go nuts¹. Remember, names are a good thing.

Once everything is composed/constructed you write one line, and one line only, of behavior code to start your object graph ticking. Something like:

$example.start();

This is the end of your procedural code. Everything else is object oriented.

You also offered:

class Example
{
    private $dependency;

    public function __construct()
    {
        $this->setDependency(new Dependency());
    }

    public function setDependency(Dependency $dependency)
    {
        $this->dependency = $dependency;
    }
}

My tests can now use this public method to set the dependency to my mocked value. This seems like the best approach, however I've added this new setDependency method just to make testing easier and that feels a bit dirty.

What makes this dirty is that it forces the class to be mutable (state can change after construction) only for the sake of testing when testing doesn't need it. Testing was perfectly happy with the first example.

But you have something here that you didn't have before. What you have here is a known-good-default-value. It's a royal pain to have to configure everything when there are obvious defaults. Convention over configuration argues that it is better to design a system with a convention of known good defaults that can be overridden by adding configuration code, not simply changing configuration code.

There is a way to give testing, convention, and configuration everything they need, all without forcing the object to be mutable:

class Example
{
    private $dependency;

    public function __construct( Dependency $dependency = new Dependency() )
    {
        $this->dependency = $dependency;
    }
}

This makes use of optional arguments². Now when you want the convention you can use:

$example = new Example();

and it's the same as if you used

$example = new Example( new Dependency() );

yet you are free to write

$example = new Example( new DependencyMock() );

in your tests or use it in your code to configure away from the convention.

Yes, technically the convention is hard coded. But the hard coded value is an overridable default value. That makes a world of difference. If you need to use factory methods this same trick works with them as well.

Lastly you hit us with this:

class Example
{
    protected $dependency;
}

And then you start talking about using reflection in your tests.

What is the right way for my tests to pass a mocked object into my Example class when I'm not using Dependency Injection?

The right way is to rewrite to accept injections. As shown above that doesn't require any fancy tools. You can use reflection to ignore encapsulation protections but it's generally better to parameterize and let the injections come in the front door rather then make them break your windows in the dark of night.

Best Answer

Related Solutions

Php – way to document required properties in traits (PHP)

Php – How to pass a mock object into a class for unit tests

Related Topic