Best practice: PHP Magic Methods __set and __get

Question

Possible Duplicate:
Are Magic Methods Best practice in PHP?

These are simple examples, but imagine you have more properties than two in your class.

What would be best practice?

a) Using __get and __set

class MyClass {
    private $firstField;
    private $secondField;

    public function __get($property) {
            if (property_exists($this, $property)) {
                return $this->$property;
            }
    }

    public function __set($property, $value) {
        if (property_exists($this, $property)) {
            $this->$property = $value;
        }
    }
}

$myClass = new MyClass();

$myClass->firstField = "This is a foo line";
$myClass->secondField = "This is a bar line";

echo $myClass->firstField;
echo $myClass->secondField;

/* Output:
    This is a foo line
    This is a bar line
 */

b) Using traditional setters and getters

class MyClass {

    private $firstField;
    private $secondField;

    public function getFirstField() {
        return $this->firstField;
    }

    public function setFirstField($firstField) {
        $this->firstField = $firstField;
    }

    public function getSecondField() {
        return $this->secondField;
    }

    public function setSecondField($secondField) {
        $this->secondField = $secondField;
    }

}

$myClass = new MyClass();

$myClass->setFirstField("This is a foo line");
$myClass->setSecondField("This is a bar line");

echo $myClass->getFirstField();
echo $myClass->getSecondField();

/* Output:
    This is a foo line
    This is a bar line
 */

In this article: http://blog.webspecies.co.uk/2011-05-23/the-new-era-of-php-frameworks.html

The author claims that using magic methods is not a good idea:

First of all, back then it was very popular to use PHP’s magic functions (__get, __call etc.). There is nothing wrong with them from a first look, but they are actually really dangerous. They make APIs unclear, auto-completion impossible and most importantly they are slow. The use case for them was to hack PHP to do things which it didn’t want to. And it worked. But made bad things happen.

But I would like to hear more opinions about this.

Answer 1

I have been exactly in your case in the past. And I went for magic methods.

This was a mistake, the last part of your question says it all :

• this is slower (than getters/setters)
• there is no auto-completion (and this is a major problem actually), and type management by the IDE for refactoring and code-browsing (under Zend Studio/PhpStorm this can be handled with the @property phpdoc annotation but that requires to maintain them: quite a pain)
• the documentation (phpdoc) doesn't match how your code is supposed to be used, and looking at your class doesn't bring much answers as well. This is confusing.
• added after edit: having getters for properties is more consistent with "real" methods where getXXX() is not only returning a private property but doing real logic. You have the same naming. For example you have $user->getName() (returns private property) and $user->getToken($key) (computed). The day your getter gets more than a getter and needs to do some logic, everything is still consistent.

Finally, and this is the biggest problem IMO : this is magic. And magic is very very bad, because you have to know how the magic works to use it properly. That's a problem I've met in a team: everybody has to understand the magic, not just you.

Getters and setters are a pain to write (I hate them) but they are worth it.

Answer 2

You only need to use magic if the object is indeed "magical". If you have a classic object with fixed properties then use setters and getters, they work fine.

If your object have dynamic properties for example it is part of a database abstraction layer, and its parameters are set at runtime then you indeed need the magic methods for convenience.

Answer 3

I use __get (and public properties) as much as possible, because they make code much more readable. Compare:

this code unequivocally says what i'm doing:

echo $user->name;

this code makes me feel stupid, which i don't enjoy:

function getName() { return $this->_name; }
....

echo $user->getName();

The difference between the two is particularly obvious when you access multiple properties at once.

echo "
    Dear $user->firstName $user->lastName!
    Your purchase:
        $product->name  $product->count x $product->price
"

and

echo "
    Dear " . $user->getFirstName() . " " . $user->getLastName() . "
    Your purchase: 
        " . $product->getName() . " " . $product->getCount() . "  x " . $product->getPrice() . " ";

Whether $a->b should really do something or just return a value is the responsibility of the callee. For the caller, $user->name and $user->accountBalance should look the same, although the latter may involve complicated calculations. In my data classes i use the following small method:

 function __get($p) { 
      $m = "get_$p";
      if(method_exists($this, $m)) return $this->$m();
      user_error("undefined property $p");
 }

when someone calls $obj->xxx and the class has get_xxx defined, this method will be implicitly called. So you can define a getter if you need it, while keeping your interface uniform and transparent. As an additional bonus this provides an elegant way to memorize calculations:

  function get_accountBalance() {
      $result = <...complex stuff...>
      // since we cache the result in a public property, the getter will be called only once
      $this->accountBalance = $result;
  }

  ....


   echo $user->accountBalance; // calculate the value
   ....
   echo $user->accountBalance; // use the cached value

Bottom line: php is a dynamic scripting language, use it that way, don't pretend you're doing Java or C#.

Answer 4

I vote for a third solution. I use this in my projects and Symfony uses something like this too:

public function __call($val, $x) {
    if(substr($val, 0, 3) == 'get') {
        $varname = strtolower(substr($val, 3));
    }
    else {
        throw new Exception('Bad method.', 500);
    }
    if(property_exists('Yourclass', $varname)) {
        return $this->$varname;
    } else {
        throw new Exception('Property does not exist: '.$varname, 500);
    }
}

This way you have automated getters (you can write setters too), and you only have to write new methods if there is a special case for a member variable.

Answer 5

I do a mix of edem's answer and your second code. This way, I have the benefits of common getter/setters (code completion in your IDE), ease of coding if I want, exceptions due to inexistent properties (great for discovering typos: $foo->naem instead of $foo->name), read only properties and compound properties.

class Foo
{
    private $_bar;
    private $_baz;

    public function getBar()
    {
        return $this->_bar;
    }

    public function setBar($value)
    {
        $this->_bar = $value;
    }

    public function getBaz()
    {
        return $this->_baz;
    }

    public function getBarBaz()
    {
        return $this->_bar . ' ' . $this->_baz;
    }

    public function __get($var)
    {
        $func = 'get'.$var;
        if (method_exists($this, $func))
        {
            return $this->$func();
        } else {
            throw new InexistentPropertyException("Inexistent property: $var");
        }
    }

    public function __set($var, $value)
    {
        $func = 'set'.$var;
        if (method_exists($this, $func))
        {
            $this->$func($value);
        } else {
            if (method_exists($this, 'get'.$var))
            {
                throw new ReadOnlyException("property $var is read-only");
            } else {
                throw new InexistentPropertyException("Inexistent property: $var");
            }
        }
    }
}

Answer 6

Second code example is much more proper way to do this because you are taking full control of data which are given to class. There are cases in which the __set and __get are useful but not in this case.

Answer 7

You should use stdClass if you want magic members, if you write a class - define what it contains.

Answer 8

The best practice would be to use traditionnal getters and setters, because of introspection or reflection. There is a way in PHP (exactly like in Java) to obtain the name of a method or of all methods. Such a thing would return "__get" in the first case and "getFirstField", "getSecondField" in the second (plus setters).

More on that: http://php.net/manual/en/book.reflection.php

Answer 9

I am now returning to setters and getters but I am also putting the getters and setters in the magic methos __get and __set. This way I have a default behavior when I do this

$class->var;

This will just call the getter I have set in the __get. Normally I will just use the getter directly but there are still some instances where this is just simpler.