13

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?

22
  • 1
    Arguably useful? The benefits you stated are extremely important. I would probably flatly refuse to work on code that had no explicit property declarations.
    – Michael
    Feb 8, 2013 at 19:01
  • 2
    Really, the way to protect yourself from these errors is with careful error checking and better, via unit testing.
    – Michael
    Feb 8, 2013 at 19:02
  • 1
    there is built in php error reporting (a notice level) that will tell you if you use a variable without first declaring it. Feb 8, 2013 at 19:05
  • 3
    @MarkAmery I would think that a frantically-paced startup is among the most important places to adopt strict unit testing - since you're always changing code, you're likely always breaking it. That's really the exact purpose of strict testing and flat-out TDD. Yeah, I guess using underscores may help you remember you're accessing private stuff, but for me the idea of adopting specific coding standards to protect me from making errors I shouldn't be making is unsettling. Like doing TRUE === $variable to prevent yourself from accidentally assigning instead of comparing.
    – Michael
    Feb 8, 2013 at 19:14
  • 1
    @MarkAmery Note also that the visibility keywords are parsed by automated documentation tools, so they give more "documentation value" than just a comment you would put in manually.
    – Michael
    Feb 8, 2013 at 19:16

2 Answers 2

16

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".

7
  • In response to your 5 downsides: 1 (Performance): Do you have a source for this? 2 (Readability): Not sure I understand; the proposal was for commenting out the declarations, not just removing them, so what readability is lost? Slightly less friendly syntax highlighting and potential difficulty distinguishing from neighboring comments, I guess? 3: This one I don't understand at all; can you clarify? 4 (IDEs): Fair enough - I don't have experience of coding PHP with an IDE and didn't know about Eclipse's type hinting. 5 (doc generators): Fair enough - never used one of these.
    – Mark Amery
    Feb 8, 2013 at 23:01
  • I didn't see your line about commenting them out. Regardless, the above points still apply - how do you know which ones are active or legitimately commented out?? Feb 8, 2013 at 23:08
  • In response to your proposed solution: this is precisely what I want to avoid - the assignment of defaults even when they are meaningless and the default values should never be used. The problem with these defaults (and with declaration without assignment, which just assigns null) is that if, due to a programming mistake, I don't assign a value to something in the constructor when I ought to, then rather than PHP giving a warning when I try to use that value later - helping me spot my error - everything will seem to work fine even though the class is broken.
    – Mark Amery
    Feb 8, 2013 at 23:11
  • 2
    @MarkAmery Your programming mistake, though, essentially amounts to a typo. We've all had them - but you're trying to detonate a bomb to kill a fly here. Feb 8, 2013 at 23:13
  • You may be right. I spent a couple of hours tracking down a bug today that turned out to be caused entirely by such a typo, and was frustrated afterwards by the sudden realization that if I'd only not used field declarations, I would've got a notice and known immediately where my error was (which I'd always thought would happen in these circumstances anyway; I didn't realize declarations initialized fields to null). The immediate accessibility of that experience is probably skewing my judgement of how likely it is to recur, or how worthwhile it is to defend against.
    – Mark Amery
    Feb 8, 2013 at 23:24
2

I worked on some code that used dynamically created object properties. I thought that using dynamically created object properties was pretty cool (true, in my opinion). However, my program took 7 seconds to run. I removed the dynamic object properties and replaced them object properties declared as part of each class (public in this case). CPU time went from over 7 seconds to 0.177 seconds. That's pretty substantial.

It is possible that I was doing something wrong in the way I was using dynamic object properties. It is also possible that my configuration is broken in some way. Of course, I should say that I have a very plain vanilla PHP configuration on my machine.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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