__set() and __get() - Further examples of poor implementation?

[seanf]

Hello all,

PHP's __set() and __get() "magic" methods appear to be another example of poor implementation of a good feature in the Zend Engine. How are you supposed to document the properties you are making available in API documentation? Also, if you need to perform different operations on values before setting or getting then you are going to end up with mammoth switch statements.

Microsoft's implementation in C# makes much more sense:

private string Whatever;
public string whatever
{
    get
    {
        return Whatever;
    }

    set
    {
        Whatever = value;
    }
} 

This allows you to document each property of the object and allows you perform any operations you need to on the value without having to always run through a switch statement first.

In the end, Zend's implementation seems to make what could be a very good feature a pretty useless one. The only real example of using these methods I can see would be where the user already knows each property they will try to access, such as a request object like the following:

<?php

    $request->get->id;
    $request->post->username;
    $request->cookie->userID;

?>

Is anyone using these methods or do people agree that this feature is largely useless due to the implementation?

Sean

[sweatje]

Marcus is currently working on a neat re-implementation of the SimpleTest MockObject code which relies on this functionionality in PHP5. Basically, if you reference $this->MockSomething in your test case, it assumes that
a) if the class MockSomething does not exists, it tries to create one
and b) it instantiates a MockSomething, and then lets you start setting expectations, etc.

This is pretty neat becuase it truely is dynamic, i.e. the test code does not know which mock objects you might be interested in setting up and using until you try to access the Mock* property.

So __get() and __set() as a replacement for public $something -1, but for possibly neat dynamic tricks where you really nead it +1

HTH

[Ezku]

Microsoft's implementation in C# makes much more sense:

private string Whatever;
public string whatever
{
    get
    {
        return Whatever;
    }

    set
    {
        Whatever = value;
    }
} 

Transparent accessor methods: I've been dreaming of that for PHP a long time. However, I don't think __get and __set are meant for that: they can do more as they'll catch a call to any property and as such allow for nice dataobjects, for example.

[Selkirk]

Its not a poor implementation. Its a very logical implementation given PHP's dynamic nature. I think you are comparing apples and oranges. __get and __set are not equivalent to properties in the sense that C# supports them. Rather, __get and __set are really handlers for a "property not found" message. PHP can use these methods to implement "virtual properties." Virtual properties are very different than the concrete properties that other languages offer.

PHP simply does not currently support concrete properties with accessor methods. Thats not to say that it never will.

You can see a more detailed explanation of my point of view in this post I made to the PHP-internals mailing list in response to a proposal to add a property keyword to php.

[mx2k]

the read only is nice too.

Public ReadOnly Property ID() As Integer
Get
return _ID
End Get
End Property


but php would have to understand the set syntax versus the get

// SET
$class->Property() = 'propertyValue';  #or
$class->Property('propertyValue'); # though, this usually what a method call looks like. 

// GET
$x = $class->Property(); 

[Ezku]

Something like this would be terrific:

class Foo
{
   private $bar;
   public get bar
   {
      return $this->bar;
   }
   public set bar(TypeHint $bar)
   {
      $this->bar = $bar;
   }
} 

[lazy_yogi]

Marcus is currently working on a neat re-implementation of the SimpleTest MockObject code which relies on this functionionality in PHP5. Basically, if you reference $this->MockSomething in your test case, it assumes that
a) if the class MockSomething does not exists, it tries to create one
and b) it instantiates a MockSomething, and then lets you start setting expectations, etc.

This is pretty neat becuase it truely is dynamic

Yeah that is a very nice feature. But Seanf's point is true. It does turn get/set properties into one enormous, unmodular, ugly switch statement.

Regards,
Eli

[Dr Livingston]

The use of SWITCH is not by all means, your only available solution, as discussed elsewhere.

I just don't understand what the problem is. Really?

[REMIYA]

I just don't understand what the problem is. Really?

Absolutely

[mx2k]

because its cleaner to have $class->Property() than $class->SetProperty() & $class->GetProperty() & its easier to document than __set() & __get

[Dr Livingston]

I don't see any difference between the two in regards to documentation? I see that we in PHP5 have __construct(); so in that event, personally I would prefer to see __set(); and __get(); also, which is more uniform.

In any case, there would be absolutely no difference in how your scripts are effected in you use __set(); or $obj -> SetProperty( ... ); except that maybe $obj -> SetProperty( ... ) is in some manner, is misleading when you have your class methods named in lower-case, ie $obj -> setProperty( ... );

As I said, __set(); and __get(); at least, are more uniform in regards that we have __construct(); and __destruct(); et al.

The arguement is academic anyways, so I still don't know what all the fuss is about

[dagfinn]

How about this?

class Object {
    protected function __get($name) {
        $method = 'get_'.$name;
        return $this->$method();
    }

    protected function __set($name,$value) {
        $method = 'set_'.$name;
        return $this->$method($value);
    }
}

class Foo extends Object {
    private $_whatever;
    protected function set_whatever($value) {
        $this->_whatever = $value;
    }
    protected function get_whatever() {
        return $this->_whatever;
    }
}


class GetSetTest extends UnitTestCase {
    function testGetSet() {
        $foo = new Foo;
        $foo->whatever = 42;
        $this->assertEqual(42,$foo->whatever);
    }
} 

[Etnu]

Hello all,

PHP's __set() and __get() "magic" methods appear to be another example of poor implementation of a good feature in the Zend Engine. How are you supposed to document the properties you are making available in API documentation? Also, if you need to perform different operations on values before setting or getting then you are going to end up with mammoth switch statements.

Microsoft's implementation in C# makes much more sense:

private string Whatever;
public string whatever
{
    get
    {
        return Whatever;
    }

    set
    {
        Whatever = value;
    }
} 

This allows you to document each property of the object and allows you perform any operations you need to on the value without having to always run through a switch statement first.

In the end, Zend's implementation seems to make what could be a very good feature a pretty useless one. The only real example of using these methods I can see would be where the user already knows each property they will try to access, such as a request object like the following:

<?php

    $request->get->id;
    $request->post->username;
    $request->cookie->userID;

?>

Is anyone using these methods or do people agree that this feature is largely useless due to the implementation?

Sean

Currently a hot topic on the internals list, actually.

While most people only see this as being a "good" thing, there's also a case where the current implementation is a lot better: namely when dealing with arrays of data, especially for a class that represents data pulled from a database which may have hundreds of rows (and declaring a getter / setter for each and every property is annoying). It's also a quick and easy way to implement read-only properties on a mass scale.

It's not perfect, but it works. The only drawback I've found with it is documentation, like was mentioned before.

[Selkirk]

This is getting off-topic, but an excerpt from WACT:

class ArrayBasedDataSourceSupport extends CommonDataSourceSupport {
    var $_properties = array();

    function set($name, $value) {
        if (isset($this->_properties[$name]) || array_key_exists($name, $this->_properties)) {
            if ($this->propertyWillChange($name, $this->_properties[$name], $value)) {
                $this->_properties[$name] = $value;
            }
        } else {
            $this->writeablePropertyNotFound($name, $value);
        }
    }
    
    function get($name) {
        if (isset($this->_properties[$name]) || array_key_exists($name, $this->_properties)) {
            return $this->_properties[$name];
        } else {
            return $this->readablePropertyNotFound($name);
        }
    } 

[mx2k]

well why not have keep the __get __set , but also provide Get Set keywords or even a property keyword?

cause i don't know if i would like not having it being able to set or get depending on which side of the equal marks it's on.

class MyClass {

     private $property;

     public function Property($value = null)
     {
           if(isset($value)){
                $this->property = $value; return;
           }
           return $this->property;
      }
} 

[Ezku]

(...)

If I'm reading this right, I understand you're trying to make property assignments look like you're assigning the value to a method. Don't know about you, but to me that seems like an extremely ugly hack. If they're going to implement accessors, they better do it properly.

How about this?

Yeah, that's a nice substitute, but I'd rather not have to hack __set and/or __get to have accessor(s) for a single property. I'm sure we all agree on this

[mx2k]

i'm saying that i don't like

$class->Property('value');

but would prefer to

$class->Property() = 'value';

i hate all the $class->setBlah('value'); $class->getBlah();
thats ugly to me. especially if people use an ide with code completion. and to me the code is not as readable.

[Ezku]

$class->Property('value');
$class->Property() = 'value';

So I did read you correctly. I'm asking again, why not do it properly the first time?

class Foo {
   private $bar;
   public get bar {
      return $this->bar;
   }
   public set bar(Object $bar) {
      $this->bar = $bar;
   }
}

$foo = new Foo;
$foo->bar = new Object;
$foo->bar = new Foo; // throws IllegalParameterException 

Not being able to tell from the outside if accessors are at play is a huge thing. In a positive way.

[lazy_yogi]

It's more about being able to use it as a property (ie data) of the class rather than method. A method is used to do something. Calling a property is not meant to do anything except get/set the field.

$obj->Run(); is obviously doing something. whereas $obj->Name is a field. The point of properties is if you want a side effect such as validation, logging, etc.

The advantage of a property is that you can validate, log, whatever, while being transparent. To increment something using $ob->setField( $obj->getField() + 1 ) is ugly and messy. Also the ability to have $obj->Count++ call the getter to get the value and the setter to set it all transparently is very nice.

I just don't understand what the problem is. Really?

The problem is that you lose modularity. If you are fine with one long method, then you are on you way to proceedural code.

[ghurtado]

What about something like this?

class Request {
    function _get($value) {
        if(get_magic_quotes_gpc()){
            return stripslashes($this->_data[$value]);
        } else {
            return $this->_data[$value];
    }
} 

I think the _get and _set methods can be tremendously useful when dealing with objects whose properties are not neccesarily known before runtime (http requests, DAOs, SQL queries, form data...) and where a certain level of filtering or checking must be done for any and all properties of the object. I can think of a few very useful examples of usage that allow you to transparently access the property of the object without even having to know what logic is being applied to it's properties before you set them or get them.

[Ezku]

I think the _get and _set methods can be tremendously useful when dealing with objects whose properties are not neccesarily known before runtime (http requests, DAOs, SQL queries, form data...)

That is most true and mentioned before. I think your example is flawed, though - wouldn't you perform the mq cleanup on populating the object?

lazy_yogi: Quite right. Transparency is the key; I understand Ruby is quite elegant in this, although I'm not experienced with it.

[ghurtado]

wouldn't you perform the mq cleanup on populating the object?

Sure, in that case change the "_get" function in the example for a "_set"; the point that it means to illustrate is unrelated to whether you do the filtering upon setting of the property or upon retrieval. In certain implementations it might even be more advisable to do the filtering as late as possible (IE: when you want to reserve a way to acquire the original unfiltered data).

[jayboots]

I agree with Selkirk. __get/__set is not merely for properties. You can implement a generic property mechanism with them which, with a good level of abstractions, is also fully documentable and does not lead to switches. The advantadge of __get/__set over get and set keywords is that the former allows for more dynamic models since you can catch all accesses in single place. I like to think of it as mapping -> syntax onto userland code rather than thinking of it as a property access facility -- not that it is foolproof in that regard.

[stereofrog]

Implementing ArrayAccess and using array notation seems to be more "natural" way to create properties.

[mx2k]

So I did read you correctly. I'm asking again, why not do it properly the first time?

class Foo {
   private $bar;
   public get bar {
      return $this->bar;
   }
   public set bar(Object $bar) {
      $this->bar = $bar;
   }
}

$foo = new Foo;
$foo->bar = new Object;
$foo->bar = new Foo; // throws IllegalParameterException 

Not being able to tell from the outside if accessors are at play is a huge thing. In a positive way.

actually you haven't read me correctly

my point is that i hate properties looking like methods...... they should be set/get depending on which the equal marks they stand on.