PHP Magic Accessors Make Me Want to Punch Babies
As many know PHP have dynamic methods (or magic methods) for producing an environment that, at run-time, is much different than one conceives at first blush. It is largely the inclusion of these magic methods that brought PHP forward and lead to many elegant and inventive solutions. These can be found in nearly every major framework and library that scatter the PHP landscape. Indeed, they let you do very crafty things; take, for example, allowing for the custom dispatch of controller actions in Zend Framework or how Doctrine uses them to perform serialized caching for objects. These examples really can really show off what PHP’s best intentions are. However, like any tool, what it was intended for, and what others are using it for, in practice, can often be at odds. My beef is with the magic accessors and the way I see them employed in many scenarios.
The use of the magic accessors (__get and __set) have allowed for the rapid growth and expansion of objects beyond anyones wildest dreams and now we are paying severely for the bloat and overhead. On top of that, often coupled with this, is the lack of validation of values following overly optimistic assumptions about the values being set. Let me show an example of such a class to make this more clear:
class User
{
protected $_data=array();
public function &__get($key)
{
$value = null;
if (isset($this->_data[$key])) {
$value = $this->_data[$key];
}
return $value;
}
public function __set($key, $value)
{
$this->_data[$key] = $value;
}
public function __isset($key)
{
return isset($this->_data[$key]);
}
public function __unset($key)
{
unset($this->_data[$key]);
}
}
What this now affords developers is the ability to, anywhere in code where they have a User object instance, create arbitrary invalidated values on the User object:
for($i=0; $i<1000000; $i++) {
array_push($user->foo, new Foo($i));
}
$user->bar = 45.352135;
$user->baz = 'I CAN HAZ ANY VALUE I WANT! MUHAHAHA!';
$user->bat = memcache_get('some_invalid_value_from_mc');
// ...
var_dump(strlen(serialize($user))); // => 134523592
My argument centers around the misuse of these magic accessors, particularly in the case where you are persisting the object (as would most likely be the case for a User object). If you have a value you are putting into an object you should be able to, in a very straightforward manner, validate it (see my post A Tailored Approach to Object Validation). What better way than to explicitly specify an instance variable and a getter and setter method for it? You can still use the magic accessors but perhaps as a catch all for values which seemed to appear out of thin air. Take, for example, the way I think most setups should be done or modeled to closely resemble:
class User
{
protected $_name;
protected $_age;
/* ... */
public function getAge()
{
return $this->_age;
}
public function getName()
{
return $this->_name;
}
public function setAge($age)
{
$minimumAge = 0;
$maximumAge = 130;
$validator = new Validator_Integer_Between($minimumAge, $maximumAge);
if (!$validator->valid($age)) {
throw new ValidationException('The age attribute must be between the values [' . $minimumAge . '] and [' . $maximumAge . ']');
}
$this->_age = $age;
}
public function setName($name)
{
$regEx = "/^[A-Za-z]+$/";
$validator = new Validator_String_Matches($regEx);
if (!$validator->valid($name)) {
throw new ValidationException('The name attribute must match the regular expression pattern [' . $regEx . ']');
}
$this->_name = $name;
}
public function __get($key)
{
throw new Exception('User::__get(' . $key . '): Attempt to get a non-existent attribute [' . $key . ']');
}
public function __set($key, $value)
{
throw new Exception('User::__set(' . $key . ', ' . $value . '): Attempt to set a non-existent attribute [' . $key . '] to value [' . $value . ']');
}
public function __isset($key)
{
throw new Exception('User::__isset(' . $key . '): Attempt to check for non-existent attribute [' . $key . ']');
}
public function __unset($key)
{
throw new Exception('User::__unset(' . $key . '): Attempt to unset a non-existent attribute [' . $key . ']');
}
}
As you can see any value that “falls through the cracks” will be caught and dealt with. In this way we are able to implement any craziness in the magic accessors, should we ever need do anything “magical”, and use concrete getters and setters so we are able to more readily (and cleanly) allow for the validation of every value set into our object, not to mention its also faster. If you have a million values in your user object you should have at least two million accessor methods (1 million gets, and 1 million sets). Adding a new attribute to the class, creating its getter method, and its setter method forces you to think about what you are doing and whether you need the value or not.





Good post, exactly what I was thinking. However, why not use the __set() function to see if the method exists, otherwise throw an exception. In your example, why not then just do:
$user = new User;
$user->age = ‘22′;
Then change the __Set to:
public function __set($key, $value)
{
if (method_exists($this, $method = “set”. $variable))
{
$this->$method($value);
}
else {
throw new Exception(‘User::__set(‘ . $key . ‘, ‘ . $value . ‘): Attempt to set a non-existent attribute [' . $key . '] to value [' . $value . ']‘);
}
}
Best of both worlds then.
I thought about that, and that is definitely a step towards using setters and getters more explicitly, but in my case I didn’t want any “magic” to happen (not to mention the performance characteristics between the two). You most certainly can do it that way, and I suppose many would, in fact, prefer it that way. In fact, this is what we ended up implementing on the project, but I wanted to go to the extreme in this post and propose the strict enforcement of being completely explicit in the values set and got. However, if this is code you maintain and you are not managing a huge project with multiple developers with a need to be super performant I would say go for it.
All of which is just a long way of saying YMMV