Re: RFC: C-sharp style property get/set syntax for PHP

On Sunday, November 28, 2010 5:18:40 pm president@basnetworks.net wrote: > Link to the RFC: > http://wiki.php.net/rfc/propertygetsetsyntax > > Thanks, > Dennis Robinson This is a very well-written and well-thought through RFC, Dennis. Nicely done. That said, I am not yet convinced. :-) First of all, I have generally found the Bean-style getter/setter approach to be a sign of poor encapsulation to begin with. You shouldn't be mucking with internal elements of an object in the first place, period. More details on that here: http://www.garfieldtech.com/blog/property-visibility However, I do see a potential use here if "properties" are treated conceptually as an entirely new concept and not as an indirection layer for class properties. That would make them more akin to a class that has both methods and also implements ArrayAccess for "properties of the conceptual object, that may not have anything to do with internal class members." Viewed from that angle, I would have the following comments: - I strongly prefer the first syntax over the second. The "property" keyword improves readability, and the closing semi-colon is going to be forgotten on a daily basis. I still forget sometimes when to use it and when not to when writing Javascript. :-) I cannot speak to the engine complexity involved but from a PHP-autor point of view I believe the first syntax is much easier to use. - The use of identical syntax for class members and properties could create confusion as to which is which. One could argue that is the point, but only if we view properties as "just" an indirection layer around the physical class members, which as I noted above I believe is a poor architectural design. There may not be an alternative here, but I mention it for completeness. - I concur with the RFP's preference for "implicit write-only" properties rather than explicit, as it seems more flexible. - The layering of accessibility keywords I find intriguing, in a mostly good way. That can offer a great deal of flexibility to control who can do what when. However, I am concerned about the confusion possible in the following: public property Hours { get { return $this->seconds / 3600; } protected set { $this->seconds = $value * 3600; } } The set method is then scoped with two different visibility directives: public and protected. Which applies? Since both are optional (presumably) I can see a potential here for confusion, especially if you also start mentioning keywords like final. This should be made more definitive if possible. - If interfaces can declare properties (niiice), Traits should be able to provide them. That provides full parallelism with methods, which I believe developers will expect. - I am curious what the performance implication would be. For instance, I've benchmarked both magic property access (__get()) and ArrayAccess to be around 4 times as slow as accessing a class member. http://www.garfieldtech.com/blog/benchmarking-magic Since in essence what is happening here is binding a function to fire when a class member is accessed (given the identical syntax), I'm concerned that there would be a similar performance issue. A 4x slower performance cost would make me avoid properties in favor of class members unless I absolutely needed them. A 2x or less I could see making more use of. - Which also brings up an interesting question: class Foo { public $a = 1; protected $b = 2; public property $a { get { return 3; } } public property $b { get { return 4; } } } $f = new Foo(); print $f->a . PHP_EOL; // Does this print 1 or 3? print $f->b . PHP_EOL; // Does this print 2 or 4, or error? I'm sure there's arguments every which way. My preference would be for properties to always win over class members, followed by the above code sample being a compile error. It gets even tricker when you introduce inheritance. If a child class has a [property|class member] of the same name as a parent's [class member| property], then what? That's all I got for now. Once again, nice RFP but still needs some thinking. --Larry Garfield