Calling the variable property directly vs getter/setters - OOP Design

Question

I know this is probably subjective but I read this optimization page from Google for PHP and they suggest use the variable property directly without the need of getters and setters. Understandably I see the performance gain in this but is this really a good design practice to follow?

Their Example using getter/setter:

class dog {
  public $name = '';

  public function setName($name) {
    $this->name = $name;
  }

  public function getName() {
    return $this->name;
  }
}

$rover = new dog();
$rover->setName('rover');
echo $rover->getName();

Suggested Optimization:

$rover = new dog();
$rover->name = 'rover';
echo $rover->name;

This would be a welcome change in my design process as I see the need for getters/setters going away, but what other hurdles/benefits might occur in doing this?

Forgetmenot

Keep in mind that this tutorial is somewhat dated. It's probably still relevant for 5.3, but the performance is really not the topic here. I wanted to recommend Berrys post on getters/setters again, but I noticed you already know it.. – Slacks

The article you linked was either written by a person ignorant of PHP internals or is really outdated. Some advice in there at least is plain wrong. It's a shame something like that can carry Googles name. – Maury

@Slacks yeah I really like Berrys post on the subject, really trying to cut down on unnecessary code. – Forgetmenot

@nikic understandably there are some things I don't agree with on the page myself, could you elaborate a little? – Forgetmenot

@Phill Pafford: The author (for I don't know what reason) thinks that $x = ...; echo $x uses twice as much memory as using echo ...; directly. – Maury

@Slacks @Phill Pafford Thanks guys, really appreciate that! Didn't know my blog post was that popular. :) – Grannias

The link to Google is now broken – Accelerometer

Answer 1

A boilerplate answer, I'm afraid but I would suggest the following: If you have no encapsulation problems for your class (enforcing business logic, etc.) by exposing this property to other users, it is perfectly ok to do so.

Bohlen

Answer 2

This would be a welcome change in my design process as I see the need for getters/setters going away, but what other hurdles/benefits might occur in doing this?

You lose the ability to implement special get/set logic on a particular property. For properties that are scalars (strings, integers, booleans) maybe this is no problem. But what if you have a property that is a lazy-loaded class instance?

class Document
{
    protected $_createdBy;

    public function getCreatedBy()
    {
        if (is_integer($this->_createdBy)) {
            $this->_createdBy = UserFactory::loadUserById($this->_createdBy);
        }
        return $this->_createdBy;
    }
}

That trick only works in a method. You could use __get and __set for this logic but as you add properties you end up with a big nasty switch() block:

public function __get($name)
{
    switch ($name) {
        case 'createdBy':
            // blah blah blah
        case 'createdDate':
            // more stuff
        // more case statements until you scream
    }
}

If you just want to avoid or put off writing getters and setters, use the __call magic method to trap method calls that follow the getProperty() and setProperty() naming convention. You can put all the default get/set logic in __call and never touch it again:

abstract class Object
{
    public function __call($method, $args)
    {
        $key = '_' . strtolower(substr($method, 3, 1)) . substr($method, 4);
        $value = isset($args[0]) ? $args[0] : null;
        switch (substr($method, 0, 3)) {
            case 'get':
                if (property_exists($this, $key)) {
                    return $this->$key;
                }
                break;

            case 'set':
                if (property_exists($this, $key)) {
                    $this->$key = $value;
                    return $this;
                }
                break;

            case 'has':
                return property_exists($this, $key);
                break;
        }

        throw new Exception('Method "' . $method . '" does not exist and was not trapped in __call()');
    }
}

This approach is very fast from a development standpoint because you can just extend the Object class, define some properties, and you're off to the races:

class Foo extends Object
{
    protected $_bar = 12345;
}

$foo = new Foo();
echo $foo->getBar();  // outputs '12345'
$foo->setBar(67890);  // next call to getBar() returns 67890
$foo->getBaz();       // oops! 'baz' doesn't exist, exception for you

It's slow from an execution standpoint because magic methods are damned slow, but you can mitigate that later by defining explicit getBar() and setBar() methods (because __call is only invoked when you calling a method that isn't defined). But if a particular property doesn't get accessed very often, maybe you don't care how slow it is. The point is, it's easy to add special get/set methods later on and the rest of your code never knows the difference.

I cribbed this approach from Magento and I find it to be very developer-friendly. Throwing an exception when calling the get/set for a property that doesn't exist helps you avoid phantom bugs caused by typos. Keeping property-specific logic in its own get/set methods makes code easier to maintain. But you don't have to write all the accessor methods at the start, you can easily go back and add them without refactoring all your other code.

The question is, what are you trying to optimize? Developer time or code speed? If you want to optimize code speed, make sure you know where your bottlenecks are before building your code around them. Premature optimization is the root of all evil.

Spiegelman

Thanks some good points. My optimizations are not about writing less code just better practices. If I need a setter/getter then that's ok but if you're just using it to set a value maybe there could be a more effective way in writing it. I'm not a big fan of the magic method, used it before. I have not tried the magic method for XMLRPC as you need to declare a docblock for all public/protected functions, could the magic method do that? – Forgetmenot

The point is that making class properties public is just another way of having default get/set logic, but it's logic that is handled by the PHP interpreter completely outside your control. And if you write a bunch of code that expects public properties it's a pain in the butt to go back later and change it to use accessor methods. As for XMLRPC, I imagine it depends on the tool you're using to handle XMLRPC calls. Could you just add docblocks without method definitions? – Spiegelman

Answer 3

This is some kind of micro-optimization. Theoretically, you can later add logic on name get/set by using magic methods (__get and __set) but practically it is not needed so much. And again, practically, this performance improvement only important only if you have everything else so optimized, that even a few microseconds add the value. In this case you can use other optimization techniques like merging all the included PHP files in one, remove type hints, decrease number of function parameters, use plain functions instead of classes. But usually adding a simple caching adds the 10-100x performance boost than all these micro-optimizations.

Tollmann

Answer 4

You could also use the __get and __set magic methods:

class Example
{
    private $allowedProps = array('prop1', 'prop2', 'prop3');
    private $data = array();

    public function __set($propName, $propValue)
    {
        if (in_array($propName, $this->allowedProps))
        {
            $this->data[$propName] = $propValue;
        }
        else
        {
            // error
        }
    }

    public function __get($propName)
    {
        if (array_key_exists($propName, $this->data))
        {
            return $this->data[$propName];
        }
        else
        {
            // error
        }
    }
}

Slickenside

From a performance standpoint, this is actually slower than using getters and setters. See this blog post: erosbence.blogspot.com/2011/07/getters-setters-performance.html – Mazzola

The performance hit is negligible. Regardless, it's still better design than just leaving things public. – Slickenside

Answer 5

At first I was surprised, I was like ... wtf. But after wrapping my brain on it a few seconds, I realized the example calls the getter function 1 million time in a loop. Of course if the variable is wrapped in a getter, we have added instructions and of course it's going to take longer.

In my opinion in most situations this is very trivial because I have yet to come accross a script that comes event close to calling getters 1 million time when running. If you do need to squeeze performance to the very last drop, it is good to know this optimisation technique.

Reverie

Answer 6

It depends on whether $name is public or not. If it's not, you can't access/modify it directly. The trade off is exposing your class's internal data elements directly to integrators. This may be OK for some classes, but not for others.

For example, you wouldn't necessarily want others to be able to modify the price of a product directly in a Product class.

Rab

Well I would probably make it protected/private unless the need to make it public is needed – Forgetmenot

If you use private, you cannot access outside of the class. Basic OO stuff. The point is, their recommended optimization only applies to public attributes. – Rab

Answer 7

I would say this is really a matter of personal preference. If performance is truly that important, then I think you answered your own question.

However, in your first example, you can still access dog::name without the getter/setter just like you do in your second example: $rover->name = 'rover'; because $name is public.

If you specifically want to hide a class member, you would need to declare the variable private or protected and then a getter/setter would be necessary.

Cutlerr