Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:63312 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 73412 invoked from network); 10 Oct 2012 03:52:33 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 10 Oct 2012 03:52:33 -0000 Authentication-Results: pb1.pair.com header.from=cpriest@zerocue.com; sender-id=pass Authentication-Results: pb1.pair.com smtp.mail=cpriest@zerocue.com; spf=pass; sender-id=pass Received-SPF: pass (pb1.pair.com: domain zerocue.com designates 74.115.204.40 as permitted sender) X-PHP-List-Original-Sender: cpriest@zerocue.com X-Host-Fingerprint: 74.115.204.40 relay-hub204.domainlocalhost.com Received: from [74.115.204.40] ([74.115.204.40:24165] helo=relay-hub204.domainlocalhost.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 0C/C1-62296-EF0F4705 for ; Tue, 09 Oct 2012 23:52:32 -0400 Received: from MBX202.domain.local ([169.254.169.44]) by HUB204.domain.local ([192.168.68.48]) with mapi id 14.02.0283.003; Tue, 9 Oct 2012 23:51:49 -0400 To: Jazzer Dane , Leigh , =?iso-8859-1?Q?Johannes_Schl=FCter_=28johannes=40schlueters=2Ede=29?= , "Rasmus Schultz (rasmus@mindplay.dk)" , "Etienne Kneuss (colder@php.net)" , "Nikita Popov (nikita.ppv@googlemail.com)" CC: "internals@lists.php.net" Thread-Topic: [PHP-DEV] [RFC] Propety Accessors v1.1 Thread-Index: Ac2lRqaw0wLAVcGGQAyyWuaNO91x4QBHSfMAABNVRgAAB5uHkA== Date: Wed, 10 Oct 2012 03:51:48 +0000 Message-ID: <9570D903A3BECE4092E924C2985CE485612B496D@MBX202.domain.local> References: <9570D903A3BECE4092E924C2985CE485612B3B48@MBX202.domain.local> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [192.168.64.22] Content-Type: multipart/alternative; boundary="_000_9570D903A3BECE4092E924C2985CE485612B496DMBX202domainloc_" MIME-Version: 1.0 Subject: RE: [PHP-DEV] [RFC] Propety Accessors v1.1 From: cpriest@zerocue.com (Clint Priest) --_000_9570D903A3BECE4092E924C2985CE485612B496DMBX202domainloc_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Wow, I'm surprised by all the talk about this RFC this time around. I post= ed this numerous times in the past trying to elicit feedback and got little= to none, so I took the time to write it as I thought it should be written.= Some of these things will take considerable effort to fix/correct/change. I'll try to address everyone's comments/questions in one post: ________________________________ Nikita: > What concerns me with the current implementation is that it leaks many im= plementation details, in particular the fact that the accessors are impleme= nted as *real* __getXYZ methods and automatic implementations also use *rea= l* $__XYZ properties. > > A few examples: > > ## 1 - __getProperty() method directly callable > > class Test { > public $property { > get { return 123; } > } > } > > $test =3D new Test; > var_dump($test->property); // int(123) > var_dump($test->__getProperty()); // int(123) > I don't particularly see this as a problem (the ability to use it as a gett= er or call it as a function), but see my comments at the end. I also don't= particularly see the point of auto implemented getters/setters ( public $h= { get; set; } ) > ## 2 - __getProperty() method exposed via exception > > class Test { > public $throwingProperty { > get { throw new Exception; } > } > } > > (new Test)->throwingProperty; > > exception 'Exception' in /home/nikic/dev/php-src/t29.php:9 Stack trace: > #0 /home/nikic/dev/php-src/t29.php(31): Test->__getthrowingProperty() > #1 {main} > I have gone to great lengths to shield the implementation details from user= s, this one slipped past me. Nearly all errors that are shown reference a = property rather than a function name and this could also be cleaned up. > ## 3 - Can directly access $__automaticProperty and even unset it (causin= g notices in the internal code) > > class Test { > public $automaticProperty { > get; set; > } > > public function getAutomaticProperty() { > return $this->__automaticProperty; > } > > public function unsetAutomaticProperty() { > unset($this->__automaticProperty); > } > } > > $test->automaticProperty =3D 'foo'; > var_dump($test->getAutomaticProperty()); > $test->unsetAutomaticProperty(); > var_dump($test->automaticProperty); > > string(3) "foo" > > Notice: Undefined property: Test::$__automaticProperty in /home/nikic/dev= /php-src/t29.php on line 13 NULL I'm not even sure that automatic backing fields are even desired, I never f= elt the need to have them in C# and the only reason they were included is b= ecause they were a part of Dennis's original proposal. Eliminating them wo= uld eliminate this as an issue. > > =3D=3D=3D=3D=3D > > I feel like this approach to the implementation will be a big can of worm= s. Sure, it works, but it is rather fragile and the enduser ends up dealing= with internal stuff which he ought not care about. I think it would be bet= ter to cleanly separate out the accessor implementation. It might require m= ore code now, but will be better in the long run. All three of these issues, could be addressed I believe by not having the f= unctions and/or properties a part of the ordinary HashTables, or to have fl= ags set on them. I believe there is already a SHADOW flag type defined whi= ch may be able to be used for this type of functionality. ________________________________ Leigh: > Further to this, take the following example. > > public $_state { > set { ... } > } > > This automatically generates a "hidden" function __set_state(), which is = a magic method already (and should be declared static, and take an array as= it's only parameter) > > Another fine example of how automatically generating *real* implementatio= ns can potentially break things. This would clearly cause problems, again this could possibly be addressed b= y not having the accessor functions be a part of the standard Functions Has= hTable, the functions would not even need to have names at that point if th= ey are not intended to be accessible directly. > public $property { > set { $this->property =3D ($this->property*2)+$value } get; } > > How do I reference the property being set from within the function? The w= ay I have done it in the example will cause recursion? How can I assign to = "self"? > > How do I set the default value of $property when the object is created? > Surely I don't have to reverse the set accessors logic and set the invers= e in __construct? Generally speaking, I don't know why you would use an automatic backing get= ter with a separate setter, but if you wanted to do that, at present you wo= uld access the automatic backing field by $this->__property. The above will not cause recursion, it is protected from recursion by the s= ame mechanism that __get() and __set() are protected. In fact, the above c= ode would set an actual property named $property on the object which would = then side-step the accessor entirely (true properties take precedence over = accessors, though they may only be "set" from within the setter of the acce= ssor. This may be a bit confusing, but I specifically wrote it this way fo= r the purpose of lazy-loading. For example: public $objList { get { return $this->objList =3D new myList(); } set { $this->objList =3D $value; } } The first access of the $objList property getter would create the object an= d attempt to set its-self which would be passed to the setter, the setter (= now guarded) will directly set the property on the object, further calls to= $objList would retrieve the already created object (bypassing the accessor= ). To get out of that situation, you would simply unset($objList) and the = accessor would take over again. Please keep in mind that this is only possible from within the setter of th= e property, nothing outside the setter of the property can create a real pr= operty named the same as the accessor, except the setter of the accessor. ________________________________ Nathan: > This would be much less confusing as it follows other PHP standards for c= reating functions and such. I know C# has a similar syntax to what is propo= sed, but we are not developing for C# we are developing for PHP which has i= ts own syntax rules that differ from C#'s and my vote is to follow PHP's al= ready in existent syntax format. I think having the function keyword there is superfluous, it adds more code= to be read, while accessors have the powers of a function, they are quite = different from a function. You would probably never have a 35 line getter,= for example. ________________________________ Rasmus: > My only remaining comment is on the read-only and write-only keywords... > this seems really superfluous and strange to me - the syntax (using a hyp= henated keyword) and the feature itself, is way off the grid as compared to= other languages. There is an important distinction with the read-only and write-only keyword= s and they are analogous to the final keyword. The final keyword prevents = any subclasses from over-riding something declared as final. Final is supp= orted for a property as well. The difference here is that a read-only prop= erty is semi-final. Subclasses may redefine a get declared as read-only bu= t may not declare a setter. Personally I am not a fan of the concepts of f= inal, read-only or write-only. They were included only because I thought t= he original RFC for which I was implementing had been ratified and the feat= ure was desired. ________________________________ Johannes: > About the items in regards to reflection in there: > > The RFC states > ReflectionClass::getMethods() will not return accessor functions > (hides implementation detail). > Up until now reflection is leaky and is telling the truth. We should eith= er keep that or completely clean up reflection. (mind also > get_class_methods() and such) I have not done anything with get_class_methods() but I certainly could. P= robably a better solution would be to have get_class_methods() use Reflecti= on to do its work, so that there is not duplicate code in the core? I see no reason to continue leaking new code, if someone wants to head up a= n effort to make the rest of Reflection not leak implementation details the= n great, but to purposely leak implementation details for new features just= doesn't make sense to me. > The RFC also introduces a new class ReflectionPropertyAccessor and has me= thods returning ReflectionProperty and ReflectionPropertyAccessor. > there should either be a common base class or interface (see i.e. > ReflectionFunctionAbstract) > I'm not certain there is a point to having a new base class, I would need t= o look at the code but I would bet that ReflectionPropertyAccessor is a sub= -class of ReflectionProperty. In any case, we have the instanceof operator= which can be used to distinguish between the results, no? > What will ReflectionPropertyAccessor->getGetter()->getName() return? I g= uess reflection will be leaky there? (see first item) I wrote the reflection additions very early on so I would need to look at t= he code, but I am pretty certain that the above would return the name of th= e property, not the function. ________________________________ Jazzer: > I think Leigh brings up some important flaws to in the current RFC. What = Leigh is asking for does not appear to be possible, and in my opinion, it s= hould be. I'm really not quite clear on what Leigh was going for with the code she in= dicated, I think she was trying to demonstrate infinite recursion which is = not possible due to guards. > I also agree with Rasmus, to a certain extent. > By putting only a getter/setter, the developer essentially sets the prope= rty as read or write only. At first glance, there is no need for the read-o= nly or write-only keyword. Except... subclassing may or may not be an issue= . > > By setting a property to read-only, it ensures that 'set' can never be se= t by a subclass. Unless I redefine the entire property... or is that not ev= en possible? The RFC isn't very clear but it appears that there is no way t= o redefine the entire property, as if you define it in the subclass with on= ly get, then it will take set, isset, and unset from the parent class, corr= ect? Though, that can be outright stopped by making the property final. But= then there is no point in having read/write-only. > I tried to be clear about this at the top of the section on read-only and w= rite-only keywords... There are three possible states: /* read-only by virtue of the fact that there is no setter, any sub-class m= ay define a setter and/or redefine the getter */ public $Hours { get { ... } } /* final, no sub-class may redefine any aspect of this accessor */ public final $Hours { get { ... } } /* Explicitly read-only, sub-classes may redefine the getter but may not de= fine a setter */ public read-only $Hours { get { ... } } As I mentioned earlier, I'm not a fan of the concept of final, read-only or= write-only, they were only included because the original RFC indicated tha= t was what the community had decided on. > From what I can tell, read-only becomes useful if I extend the class and = want to partially modify the property. > Example: > class A { > public $seconds =3D 3600; > > public $hours { > get() { return $this->seconds / 3600 }; > } > } > > class B extends A { > public $hours { // Maintains 'get' from class A > set($value) { $this->seconds =3D $value; } > } > } > > ^There's no way to stop the developer from doing that without read-only. That is correct > > Also, if the property is public, what if an outside class tries to do thi= s: > class A { > public $seconds =3D 3600; > > public $hours { > get() { return $this->seconds / 3600 }; > } > } > > $object =3D new A(); > $object->hours =3D 100; > What happens then? A fatal error occurs as there is no setter for hours. It is a read only pr= operty by virtue of the fact that it has no setter. A sub-class could defi= ne a setter for $hours if it so desired (due to the lack of a final or read= -only keyword in the declaration). > > And similarly, how do we set a public property as a property accessor, hm= m? > class A { > public $hours =3D 1; > } > > $seconds =3D 20; > > $object =3D new A(); > $object->hours =3D { // Does this work? > get() { return $seconds; } > }; I'm not quite sure what you mean here, are you asking if you can dynamicall= y define a property? The answer is no, though I think that would be cool i= t is likely well beyond my ability to accomplish being that this is my firs= t contribution to the php project. ________________________________ David: > class MyClass{ > > public $property =3D 5 //?can we do this? > { > set($value){ > if(!is_int($value)){ > throw new InvalidArgumentException('value of wrong type'= ); > } > $this->__property =3D $value; > } > } > } > > Or would that have to happen in the constructor? > > class MyClass{ > > public $property > { > set($value){ > if(!is_int($value)){ > throw new InvalidArgumentException('value of wrong type'= ); > } > $this->__property =3D $value; > } > } > > public function __construct(){ > $this->property =3D 5; > } > } I think there is some wide-spread confusion over what an accessor is. An a= ccessor does not store its own value, there is no "memory space" allocated = for an accessor unless you use the automatic implementation ( public $h { g= et; set; } ) but if you're going to do that, you may as well just reduce it= to ( private $__h ) which is all the previous "accessor" would have done f= or you. Accessors, in my own use, are nearly always used for convenience aliases wi= th conversions ( $Hours calculated based on a real stored $Seconds value) o= r as an alias to access a value from another class, they are generally not = used to simply store or retrieve values, that's what a property is for, an = accessor is for executing code with the syntactic sugar of making it seem l= ike it's a variable. ________________________________ In closing for all of this, I really wish that this type of discourse could= have happened long ago when I was requesting it, because many, many hours = have been put into the current incarnation. I would like to put together = a "vote grid" or something on all that has been discussed here and come to = a consensus before I go and re-do any further work. Does the php community= have any voting facilities or is it still done via the wiki pages? How sh= ould I decide who gets a vote and who doesn't? Should I even be handling t= he voting myself? --_000_9570D903A3BECE4092E924C2985CE485612B496DMBX202domainloc_--