Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:63304
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.42 as permitted sender)
MIME-Version: 1.0
Sender: ekneuss@gmail.com
In-Reply-To: <CAF+90c9eTXLj5+F5KY2rdFy4zC=qkETEcBcdJfr5Of_63XCgAw@mail.gmail.com>
References: <9570D903A3BECE4092E924C2985CE485612B3B48@MBX202.domain.local> <CAF+90c9eTXLj5+F5KY2rdFy4zC=qkETEcBcdJfr5Of_63XCgAw@mail.gmail.com>
Date: Tue, 9 Oct 2012 15:15:01 +0200
Message-ID: <CACQesk6vPzDzMdeC3oKRgmSfYpe-MB_0v1cTPu04MOoyPVYHWw@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Clint Priest <cpriest@zerocue.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Propety Accessors v1.1
From: colder@php.net (Etienne Kneuss)

Hi,


On Tue, Oct 9, 2012 at 3:01 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> 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.
>

I disagree, to me that this feature is all about syntactic sugar, as
such it does what it expected: it generates concrete properties and
methods that are somewhat hidden to the end-user.

I feel that any implementation that do not rely on proper
properties/methods would be a big hack.

Best,

> Nikita
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>



-- 
Etienne Kneuss
http://www.colder.ch