Re: [RFC] Propety Accessors v1.1

On Mon, Oct 8, 2012 at 1:52 PM, Clint Priest <cpriest@zerocue.com> wrote: > It's been a while since I posted any updates about this, a few individuals have been > asking about it privately and wanting me to get it out the door for PHP 5.5 release. It's come > a long way since the last time I posted about it. > > RFC Document: > https://wiki.php.net/rfc/propertygetsetsyntax-as-implemented > > Example Usage: > > class TimePeriod { > private $Seconds = 3600; > > public $Hours { > get { return $this->Seconds / 3600; } > set { $this->Seconds = $value; } > isset<http://www.php.net/isset> { return > isset<http://www.php.net/isset>($this->Seconds); } > unset<http://www.php.net/unset> { > unset<http://www.php.net/unset>($this->Seconds); } > } > } > > Changes / Updates > > * isset/unset accessor functions now implemented (object & static context, auto > implementations, etc) > > * static accessor now fully functional > > * Reference functionality validated, tests written > > * All operators have been tested, tests written > > * read-only and write-only keywords: Added explanation of reasons for inclusion at the > top of the appropriate RFC section > > * Tested for speed, approaches or meets __get() speed. > > Internally things have changed quite a bit > > * cleaned up and simplified > > * had been using 4 to 5 additional fn_flag slots, now down to two (READ_ONLY and > WRITE_ONLY) > > * the automatic implementations now compiled internal php code, this greatly simplified > that part of the code and future proofed it. > > The code is available at the url below and is up to date with master, all tests pass. > https://github.com/cpriest/php-src > > I'd like to get this project wrapped up in time to make it to the 5.5 release, only a few > things remain to be completed/updated: > > * Check on reflection code written prior to major changes (tests still pass) > > * Add a few more reflection functions that were requested > > In total there are 79 tests for this new functionality, if there are any others that I have > missed, please let me know. What concerns me with the current implementation is that it leaks many implementation details, in particular the fact that the accessors are implemented as *real* __getXYZ methods and automatic implementations also use *real* $__XYZ properties. A few examples: ## 1 - __getProperty() method directly callable class Test { public $property { get { return 123; } } } $test = new Test; var_dump($test->property); // int(123) var_dump($test->__getProperty()); // int(123) ## 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} ## 3 - Can directly access $__automaticProperty and even unset it (causing 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 = '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 feel like this approach to the implementation will be a big can of worms. 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 better to cleanly separate out the accessor implementation. It might require more code now, but will be better in the long run. Nikita