3

Other similar questions:

Why I should use __get() and __set()-magic methods in php?
When do/should I use __construct(), __get(), __set(), and __call() in PHP?

First, I completely understand how to implement __get and __set methods in PHP and have come across scenarios where using these methods is beneficial.

However, I have encountered, and written, code that looks like the following:

class SomeObject {

    protected $data = array();

    public function __set($key, $val) {
        $this->data[$key] = $val;
    }

    public function __get($key) {
        return $this->data[$key];
    }

}

Why do we need to do this? I have used objects with no __get or __set methods defined and can still add undefined properties, retrieve the values associated with those properties, invoke isset() and unset() on those properties.

Do we really need to implement these magic methods, using code like above, when objects in PHP already exhibit similar behavior?

Note, I'm not referring to when more code is used in the __set and __get methods for special handling of the data requested but very basic implementation like the code above.

Community
  • 1
  • 1
Charles Sprayberry
  • 7,741
  • 3
  • 41
  • 50
  • Guessing the intent? Anticipation of actual consolidation logic for property access. And their general use is for eschewing [not very OOish](http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html) getXy/setXy methods. But empty ones are obviously just as pointless. – mario Dec 03 '11 at 01:46

6 Answers6

4

If you are using them just as a glorified wrapper over an array (i.e. absolutely no extra logic), I don't believe they offer any benefit. They do offer a drawback however, in that you need to write the code for them. And that code may be buggy or incomplete.

Did you leave out __isset for brevity? Because if you do use __get and __set but do not provide an __isset, you 've just written yourself a bug.

See what I did there?

Jon
  • 428,835
  • 81
  • 738
  • 806
  • Yes, I left out the `__isset` for brevity. In all of my implementations I provide `__isset` and `__unset` as well. – Charles Sprayberry Dec 03 '11 at 01:36
  • I definitely agree with your statement on forgetting to implement `__isset()`. I was recently pulling my hair out over `empty()` returning an incorrect result, as I forgot to include an `__isset()` method. –  Dec 03 '11 at 01:37
  • @TimCooper: Mr. Hair Pulling has taught me as well. Only in my case it was more like [The Cruel Tutelage of Pai Mei](http://killbill.wikia.com/wiki/Chapter_8:_The_Cruel_Tutelage_of_Pai_Mei), since when it happened there *was* no `__isset` to override. – Jon Dec 03 '11 at 01:41
2

In the example you gave, it's very easy to overwrite $data with a new array. If you were dealing with properties, you would have to iterate over an array and perform $this->$key = $data; (to which you may have data leakage).

In more detail:

protected function setData($arr)
{
   $this->data = $arr;
}

vs.

protected function setData($arr)
{
    foreach($arr as $key => $data)
    {
        $this->$key => $data;
    }
}

With the example above, keys which don't appear in $arr in the current call, but have in previous calls, will not be overwritten, which can lead to undesired results.

In the end, it just depends on what makes the most sense for you.


As a side note, in the example code you gave, you should be sure you're implementing __isset() when you have __get(). If you don't, you'll get unexpected results when using functions like isset() or empty().

2

To me the sample code looks like "We want all the downsides of an object combined with all the misuse potential of an array"

The only reason I can see for going for something like that is when you are starting to refactor from arrays into objects.

You replace all [''] access with -> but you don't break anything once it compiles again.

In the next step you can start adding rules for specific properties.

public function __set($key, $val) {
    if($key == "money") { throw new Exception("Can't touch this"); }
    $this->data[$key] = $val;
}

public function __get($key, $val) {
    if($key == "money") { return $this->x * $this->y; }
    $this->data[$key] = $val;
}

But in general I don't see any use going in that direction. Creating real "value objects" for OO data structures seems a lot more useful.

Doing something like that just for the sake of "I like -> better than [''] isn't a valid use case in my book."


For most use cases with "Objects as data structures" I'd much rather have:

class SomeObject {

    protected $a, $b, $c;

    public function __construct(TypeA $a, TypeB $b, TypeC $c) {
        $this->a = $a;
        $this->b = $b;
        $this->c = $c;
    }

    /** @return TypeA */
    public function getA($key) {
        return $this->a;
    }
    // an so on

}

That documents the expected values. Works with native type hinting without the need of writing that your self and to me is what I'd expect to get as a "data structure".

If there is a good reason that a class should act like an array then ArrayAccess seems like a much better choice then that kind of magic to me.

edorian
  • 38,542
  • 15
  • 125
  • 143
1

There are quite a few different reasons you might want to use this, it all depends on how you want to control your class. A couple of examples:

1.) You have some attributes on your object that are related to a key -> value database. If more values are added, you would like to update the database with these. So in this method, you would have some sort of flag that indicates there is a $new_data array that needs to be synchronized with the database

2.) You want to strictly enforce that an object will only have the variables attached to it that you want to be there. If one doesn't exist for it, this can throw an exception.

3.) You've abstracted a database such that gets/sets will have an immediate impact on their corresponding values in it.

And many others.

jprofitt
  • 10,874
  • 4
  • 36
  • 46
0

In your example, no, you don't.

However, when we go into more complicated classes, you may want to define setters and getters. A sample __set would then be :

function __set($key, $val) {
    if (method_exists($this, 'set'.ucfirst($key))) {
        $this -> {'set'.ucfirst($key)}($val);
    } else {
        $this -> data[$key] = $val;
    }
}

With this code you can simply define a function setTestKey which will set $this->data['testKey'] to the value in $value, and call it via $storage->testKey = 1;

Tom van der Woerdt
  • 29,532
  • 7
  • 72
  • 105
0

I'm with you, I've never been a fan of magic methods because I feel that it breaks encapsulation. If you allow calling code to dictate object behavior that the object isn't aware of you risk un-expected behavior, which is harder to track down compared to having static setters/getters accessing defined member variables.

With that being said I have seen some clever instances where magic methods appear to be useful. One instance is calling a logger and defining the loglevel in which you want the message to appear.

Consider the following (I use this in my production code):

// Only works with php 5.3 or greater (uses __callstatic)
class MyCustom_Api_Log
{
    /**
     * Wrapper method for getting a symfony logger object
     *
     * @return object
     */
    public static function getLogger()
    {
        return sfContext::getInstance()->getLogger();
    }

    /**
     * Magic method for logging all supported log levels
     *
     * @param string
     * @param array
     */
     public static function __callStatic($level, $params)
     {
         // Only concerned with message at this time
         // However any number of params can be passed in
         $message = shift($params);

        self::getLogger()->$level($message);
     }
}

// All these calls are made to __callStatic, so no need for individual methods
MyCustom_Api_Log::err(array('Oh noes something went wrong'));
MyCustom_Api_Log::warn(array('Oh noes something went wrong'));
MyCustom_Api_Log::notice(array('Oh noes something went wrong'));

As you can see, I can call the magic method with err, warn, or notice and logs my messages at that log level.

Mike Purcell
  • 19,847
  • 10
  • 52
  • 89