Best practice: PHP Magic Methods __set and __get [duplicate]

Question

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.

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.

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

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");
            }
        }
    }
}

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.