Do you use setters and getters within a class as well?

[mgkimsal]

The whole 'why don't you think before asking?' comment was a bit harsh, to say the least. I probably could have emphasized 'more' instead of 'careful', in my earlier post, but I still don't see why people would have to be 'more' careful. Either you're careful about checking to access to undefined properties or you're not.

What are those 'good programming practices' ?

I would say having a class with all the properties defined in the class definition is a good thing.

  class foo {
     
     public $bar = 1;
     public $baz = 2;
     private $zaz = 3;
     
     } 

Throwing in some phpdoc on top of that is even nicer. However, if I write my code this way, I can't deal with $bar, $baz or $zaz using the __set and __get functionality.

I'm not sure why this is being overlooked. Throwing all accesses to a missing property into one defined array is the only way I've seen it done, but it feels very much like a hack, at best.

Why do you consider __set/__get as poor programming practice ?
What does __set/__get have to do with variable initialization ?
I fail to see it.

See above.

Please provide examples, as we are talking about computer science here and not literature.

What on earth is that 'literature' reference to?

I've worked places what use E_ALL in production just to catch people who don't initialize $foo before looping an array and appending via "$foo.=$bar;". In the strictest sense, that's considered 'bad' because you didn't start by stating $foo=''; to initialize the variable. So, defining and initializing values is (apparently) considered a good thing. But you can't do that in a class definition and still have access to those properties via __get and __set functionality. Not sure what's so difficult to understand here. Apparently quite a lot of people *like* it this way, although it just seems wrong to me.

As someone else pointed out, this is fancy error handling, but it's forcing people in to creating code which would throw errors in the first place (referencing undefined properties) instead of being a more integral part of the object system which allows interception of any property access, defined or undefined.

[byron3@earthlink]

Wow some of you can really muddy the water in what should be a very clear conversation. I deal in real world objects that need to be precise with valid data. Below is a small class used with a UPS Rates interface.

[code]
class Box {
    
    private function __construct() {
        $this->Heigth = 10;  // variable for box heigth
        $this->Length = 10;  // variable for box length
        $this->Weight = 10; // variable for box weight
        $this->Width = 10; //varable for box width
    }
    
    public function __get($name) {
        $value;
        $value = $this-$name;
        return $value;
    }
        
    public function __set ($name, $value) {
        if ($name = "Weight") {
            if ( is_numeric($value) && $value >= 0.1 && $value <= 150.0 )
                $this->Weight = $value;
            else
                throw new Exception ( 'Value is not numeric or falls outside of acceptable parameters', 800);
        }
        elseif ( $name == 'Heigth' || $name == 'Length' || $name == 'Width' ) {
                if ( is_numeric($value) && $value >=1 && $value <= 108.00 )
                    $this->$name = $value;
            else
                throw new Exception ( 'Value is not numeric or falls outside of acceptable parameters', 800);
        }
        else
            throw new Exception ( 'Not a valid property of Box Class', 790 );
    }
} // End box class
[/code] 

The get and set functions do just exactly what needs to be done, validate data, controll user access and are easily understood by me or another programmer using this object. I will trade off a few milliseconds for the ability to check my data any day. If you programmers are using all public varables for access to object propertys where does your data validation occur? In todays high speed server environment is erroneous data worth something undetectable to the average client?

My only complaint about variables accessed through get or set methods is that my Komodo editor does not catch them for auto completion, but is this a php problem or one with Komodos design?

[fantasma]

I have a tendancy to create accessors in my classes due to my previous experience with Java, even though the use of private variables in PHP is limit to PHP5+. Just an old habit.

To be honest I see little difference between using accessors and not using them. Sure it's makes the code a little bit bloated but in my opinion it makes it a little more obvious as to what's happening, and maybe a little more maintainable if PHP is going down the private variable route.

[megamanXplosion]

Yes, I do use getters and setters within the class itself if the getter/setter does more than retrieve the variable. The whole point of using getters and setters, in my opinion, is to ensure the data you send and retrieve is safe for usage. Any security problems found with a getter/setter class are contained within one or two functions (getter and/or setter) and the vulnerability can be quickly fixed. Without the getter/setter method of working with variables, you introduce the possibility of having a single vulnerability spread throughout the entire class - which increases the work involved in patching the vulnerability and increases the chance of missing code that does need to be patched.

[OfficeOfTheLaw]

Using __get and __set may be okay for small objects, but I find it somewhat stupid to program that way as it is QUITE a hack. It's inviteable that you'll wind up with a large switch statement to set values and enforce domain logic in one big method. And how the heck are you supposed to generate documentation when you have no class member variables defined?

No thanks.

[byron3@earthlink]

James what part of document in your class header or document in your construct function as you initalize your attributes do you not agree with? True in php all get or set methods fall into one large block of code, Maybe in php6 the zend engineers will go to a seperate set/get set of functions for each attribute as occurs in Visual Basic. The Microsoft engineers saw data validating and strict type casting as being a neccessity, something that publicly declared attributes in php seem to be lacking. While php still has some more polishing to go through the methods provided are the best we have to work with at this time. I still want to know at what point in your datas life cycle do you validate data before passing it to an object? How do you make your attributes read / write only? The language is not perfect but I have to think the great Zend engineers who have many more hours of programming time and insight had a reason for the get / set functions as they stand now.

[mgkimsal]

James what part of document in your class header or document in your construct function as you initalize your attributes do you not agree with?

I think what he's meaning is that if you initialize them via 'public' or 'var' or whatever in the class, they won't get processed by the __get and __set 'magic' methods.

The language is not perfect but I have to think the great Zend engineers who have many more hours of programming time and insight had a reason for the get / set functions as they stand now.

Many more hours of programming than who? Perhaps the reason is they didn't know any better, or that they are learning as they go along. Everyone has to learn, and some people do it in public. I'm not saying PHP devs specifically DIDN'T know what they were doing, but just because it exists in the language does not mean it's correct or even good.

[OfficeOfTheLaw]

James what part of document in your class header or document in your construct function as you initalize your attributes do you not agree with? True in php all get or set methods fall into one large block of code, Maybe in php6 the zend engineers will go to a seperate set/get set of functions for each attribute as occurs in Visual Basic. The Microsoft engineers saw data validating and strict type casting as being a neccessity, something that publicly declared attributes in php seem to be lacking. While php still has some more polishing to go through the methods provided are the best we have to work with at this time. I still want to know at what point in your datas life cycle do you validate data before passing it to an object? How do you make your attributes read / write only? The language is not perfect but I have to think the great Zend engineers who have many more hours of programming time and insight had a reason for the get / set functions as they stand now.

The thing I do not like about them (although some may find them convenient) is that I perfer to keep my methods small and to the point. Let's say that each set method requires about 5 to 10 lines of code for some kind of domain logic. if there are 10 attributes in the class that require this, you will wind up with one big monster method with 50 to 100 lines of code.

For the most part, I use __get and __set just for handling attempts to set a public attribute that isn't declared and that shouldn't be part of the object. I know there are some who like polymorphism and "one magic object that handles variable attributes" at the center of their application, but I prefer to keep things concrete to capture the business logic.

Of course, validation doesn't occur in the data object in most instances, but rather there's other "strict" validation. If I only want DummyClass::$foo to be an object of type Foo, I'd have the appropriate setter:

public function setFoo(Foo $foo)
  {
  $this->foo = $foo;
  } 

Or perhaps I don't even want to just directly set it like that. What if, as per the specifications, a variable can only be one of 10 possible values and should throw an exception on an invalid value being set (basically, an Enum)?

public function setSomeString(Foo $foo)
  {
  $this->foo->setValue($foo);
  } 

Regardless, we are way off course. This is not an arguement about using __set or __get, but rather, this is about using setters and getters to get/set private/protected variables and using them internally. If you are pulling that dynmanic crap and using __get and __set, the point is moot because your attributes will ALWAYS be public, and therefore I guess you don't need a setter or getter anyway.

[Selkirk]

I like where you are going with this, and I think this would really help solve the problems within a class. But what about things like DI or Persistence where you want to intercept accesses (or calls) but you won't know what the properties or methods will be. Can you just do:

public property get getAll set setAll;

There is no need for this because thats essentially __get and __set.

Although, one design decision would be whether or not to include the property name as a parameter to the accessor method. if you do, then accessor methods can be shared:

public property height get $_height set setDimension;
public property width get $_width set setDimension; 

On the other hand, if you include the property name as a parameter, they get slower and more complicated.

[Ezku]

Just copy the object model from Ruby and be done with it. Virtual properties, proper accessors and operator overloading, I welcome thee.

[OfficeOfTheLaw]

Just copy the object model from Ruby and be done with it. Virtual properties, proper accessors and operator overloading, I welcome thee.

I knew it wouldn't be long before this thread got hijacked by the Ruby Enthusiasts.

Where's the obligatory "See, in ruby, all you have to do is [insert code sample] which would be much nicer if php would just do this."?

[Ezku]

I knew it wouldn't be long before this thread got hijacked by the Ruby Enthusiasts.

I'm not hijacking, I'm not a Ruby enthusiast and neither am I joking. Ruby has proper accessors and error handling functions for "dynamic" properties. The latter we have in PHP, but crippled; last time I checked, this resulted in a fatal:

class Foo
{
   protected $property;
   public function __get($name)
   {
       if($name == 'property') return $this->property;
   }
}
$foo = new Foo;
$foo->property; 

The fact that Ruby has something and PHP doesn't is not an argument for not implementing it in PHP; more on the contrary, although not per se.

Dear PHP developers, give me something like this to work with:

class Foo
{
   protected $bar;
   public get virtual
   {
      return $this->bar;
   }
   public set virtual(Bar $bar)
   {
      $this->bar = $bar;
   }
   public function __get($name)
   {
      throw new CustomException('Oops.');
   }
}
$foo = new Foo;
$foo->virtual = new Bar;
$foo->virtual;
$foo->bar; // throws CustomException 

Don't cling to the details too much. I just want
a) transparent accessors and
b) catching calls to undefined properties and, preferably,
c) virtual properties via accessors (as in the example)

The benefits are obvious and thus omitted from this post.

[arborint]

There is no need for this because thats essentially __get and __set.

Although, one design decision would be whether or not to include the property name as a parameter to the accessor method. if you do, then accessor methods can be shared:

public property height get $_height set setDimension;
public property width get $_width set setDimension; 

On the other hand, if you include the property name as a parameter, they get slower and more complicated.

You still need some kind of capabilty of intercepting access to defined properties (and functions) when you don't know their names (i.e. parent). I agree that performance is very important.

[BerislavLopac]

I just thought of a simple way to emulate properties in PHP5 -- actually I've been doing it for a while without realizing that.

Make an attribute of the class to be an array, and then get and set them through __set and __get; here's a simplest implementation:

class ClassWithProperties
{
    private $properties = array();

    public function __get($key)
    {
        return isset($this->properties[$key]) ? $this->properties[$key] : null;
    }

    public function __set($key, $value = null)
    {
        if(is_null($value)) {
            if(isset($this->properties[$key])) {
                unset($this->properties[$key]);
            }
        }
        else {
            $this->properties[$key] = $value;
        }
    }

} 

[mgkimsal]

I just thought of a simple way to emulate properties in PHP5 -- actually I've been doing it for a while without realizing that.

This is demonstrated in a bit more detail at
http://us3.php.net/manual/en/languag...verloading.php

Doing this means that you can't have any of your properties defined in the classic sense with "public", "private", etc. in the class definition itself.

[BerislavLopac]

Doing this means that you can't have any of your properties defined in the classic sense with "public", "private", etc. in the class definition itself.

I agree, not in the classic sense.

But any properties defined this way would be public; private and protected properties could be set via classic setters and getters.

[bonefry]

I probably could have emphasized 'more' instead of 'careful', in my earlier post, but I still don't see why people would have to be 'more' careful. Either you're careful about checking to access to undefined properties or you're not.

When I was speaking of 'more carefull', I was speaking of the extra attention you have to pay to `fixed` properties ... i.e. you have to be carefull not to throw a 'missing property' error when that property exists ... and as I tried to demonstrate ... that's an easy thing to do ... overlooking existing fixed properties.

I would say having a class with all the properties defined in the class definition is a good thing.

  class foo {
     
     public $bar = 1;
     public $baz = 2;
     private $zaz = 3;
     
     } 

Throwing in some phpdoc on top of that is even nicer. However, if I write my code this way, I can't deal with $bar, $baz or $zaz using the __set and __get functionality.

I'm not sure why this is being overlooked. Throwing all accesses to a missing property into one defined array is the only way I've seen it done, but it feels very much like a hack, at best.

My friend, you are talking about only one way to handle this.
Keeping undefined properties in an array is not the only way to go.

As I demonstrated earlier, you can use __set/__get + reflection to build a C#-like setter/getter system.

Because someone complained it is slow (I'm too lazy to search for that message now) I will give a more simple sollution which doesn't use the complex and slow reflection framework in PHP5:

class TimeTask
{    
    private $_timer = null;
        
    function __set($property, $value)
    {    
        $methodName = 'set'.(strtoupper(substr($property,0,1)).substr($property,1));
        if (!method_exists($this, $methodName))
            throw new Exception("$property setter is not defined");
            
        $this->$methodName($value);
    }
    
    function __get($property)
    {
        $methodName = 'get'.(strtoupper(substr($property,0,1)).substr($property,1));
        if (!method_exists($this, $methodName))
            throw new Exception("$property getter is not defined");
            
        return $this->$methodName();
    }
    
    function setTimer($value)
    {
        $value = (int)$value;
        if ($value <= time())
            throw new Exception("Cannot set time in the past");
        $this->_timer = $value;
    }
    
    function getMysqlDateTime()
    {
        return date('Y-m-d H:i:s', $this->_timer);
    }
}

$t = new TimeTask();

$t->timer = time() + 10;

echo $t->mysqlDateTime;

// this will throw an exception
echo $t->unknownProperty; 

What is so wrong with this ? You can definitelly find the setter/getters through reflection as methods. You can definitelly add them PHPDOC comments.

And in case you were wondering ... C# does the same thing ... it translates setters/getters to methods named set__Property / get__Property .

[Ezku]

What is so wrong with this ?

If you ask me: it's hacky and very not-neat. Works as long as no-one goes and asks for $_timer, I'll give you that.

You can definitelly find the setter/getters through reflection as methods. You can definitelly add them PHPDOC comments.

Both valid points.

And in case you were wondering ... C# does the same thing ... it translates setters/getters to methods named set__Property / get__Property .

This was an interesting tidbit, taking into account the syntax in C#:

class Bar
{
   private string foo;
   public string foo
   {
      get { return foo; }
      set { foo = value; }
   }
} 

[BerislavLopac]

This is demonstrated in a bit more detail at
http://us3.php.net/manual/en/languag...verloading.php

Where I just found this: "Since PHP 5.1.0 it is also possible to overload the isset() and unset() functions via the __isset and __unset methods respectively." Nice.

[mgkimsal]

Bonefry,

I appreciate the fuller example you just gave. If it works for you, that's great. That seems a bit convoluted, as you seem to have to make your properties private, and you would seem to lose the distinction between private and public properties.

Anyway, as I said, if this works for you and you are productive with this, that's fine.

[Brenden Vickery]

What is so wrong with this?

Nothing that I can see. At this point the question becomes do I want to add those __get and __set methods in every class (or in a base class) so I can access properties like this, $object->property, instead of this, $object->getProperty(). The answer for me is no.

[DougBTX]

    function __set($property, $value)
    {    
        $methodName = 'set'.(strtoupper(substr($property,0,1)).substr($property,1));
        if (!method_exists($this, $methodName))
            throw new Exception("$property setter is not defined");
            
        $this->$methodName($value);
    } 

I like it.

Remember that PHP is caseinsensitive, so you can drop the substrs:

class TimeTask
{    
    private $_timer = null;
        
    function __set($property, $value)
    {    
        $method = 'set'.$property;
        if (!method_exists($this, $method))
            throw new Exception("$property setter is not defined");
            
        $this->$method($value);
    }
    
    function __get($property)
    {
        $method = 'get'.$property;
        if (!method_exists($this, $method))
            throw new Exception("$property getter is not defined");
            
        return $this->$method();
    }
    
    function setTimer($value)
    {
        $value = (int)$value;
        if ($value <= time())
            throw new Exception("Cannot set time in the past");
        $this->_timer = $value;
    }
    
    function getMysqlDateTime()
    {
        return date('Y-m-d H:i:s', $this->_timer);
    }
}

$t = new TimeTask();

$t->timer = time() + 10;

echo $t->mysqlDateTime;

// this will throw an exception
echo $t->unknownProperty; 

And this really should be in a standard library somewhere so we can extend from it:

class PObject
{
    function __set($property, $value)
    {    
        $method = 'set'.$property;
        if (!method_exists($this, $method))
            throw new Exception("$property setter is not defined");
            
        $this->$method($value);
    }
    
    function __get($property)
    {
        $method = 'get'.$property;
        if (!method_exists($this, $method))
            throw new Exception("$property getter is not defined");
            
        return $this->$method();
    }
} 

Perhaps it'll be in ZObject? Who to recommend it to?

Douglas

[stereofrog]

And this really should be in a standard library somewhere so we can extend from it:

The problem is (as someone already mentioned) that __get/__set are not reenterable, which IMO makes the whole feature pretty useless. In fact, arrow notation for 'virtual' properties is simply dangerous, because this will fail miserably if gets into __get execution chain.

Illustration

class A {
    function __set($property, $value) {    
        call_user_func(array($this, "set$property"), $value);
    }
    function __get($property) {
        return call_user_func(array($this, "get$property"));
    }
} 

class B extends A {
    function getX() { return 10;}
    function getDoubleX() { return $this->x * 2; }
}

$b = new B;

echo $b->x;  // this works
echo $b->doubleX; // this doesn't 

[arborint]

The problem is (as someone already mentioned) that __get/__set are not reenterable, which IMO makes the whole feature pretty useless. In fact, arrow notation for 'virtual' properties is simply dangerous, because this will fail miserably if gets into __get execution chain.

This gets back to the Container thread. Because of the quirks in __get/__set, the design that currently can do everything PHP4 and PHP5 is:

class Keyed {
    function get($key)
    function set($key, $value)
} 

You can: set an existing property, create a non-existent property, error on a non-existent property, store values in some other var such as an array, or call a "get$key()" method. You just can't do $obj->key = $value;

[Ezku]

class B extends A {
    function getX() { return 10;}
    function getDoubleX() { return $this->x * 2; }
}
$b = new B;
echo $b->x;  // this works
echo $b->doubleX; // this doesn't 

Could this be a version related issue? They fixed chaining virtuals ($a->b->c, where b and c are virtual) in 5.0.5, and I've no reason to believe your example wouldn't work; I believe I've done it in several occasions.