Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:65047
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <7B.46.25745.19D5CF05@pb1.pair.com>
To: internals@lists.php.net
Date: Sun, 20 Jan 2013 22:11:42 +0100
Reply-To: gooh@php.net
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130107 Thunderbird/17.0.2
MIME-Version: 1.0
References: <50F840F4.7080704@zerocue.com>
In-Reply-To: <50F840F4.7080704@zerocue.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [VOTE] Property Accessors for 5.5
From: gooh@php.net (Gordon Oheim)

Am 17.01.2013 19:20, schrieb Clint Priest:
> I'm happy to say that Property Accessors is ready for a vote for
> inclusion in 5.5 release.
>
> Nikita and I (as well as Stas a bit) have all been working hard to make
> this happen for 5.5, voting and the specifications are here:
>
> https://wiki.php.net/rfc/propertygetsetsyntax-v1.2#voting
>
> Thanks,
>
> -Clint

Thanks Clint and Niki for your work. Sorry to respond only now that the 
voting process has already started. I usually don't follow/participate 
on internals due to the noise and poisoned atmosphere (but that's a 
different story). Also thanks Niki for taking the time to answer all my 
questions I had about this beforehand.

IMO, the new getter/setter syntax adds very little benefit for the loads 
of syntax it introduces. I understand that people are looking forward to 
that new feature though, so I won't oppose it just because I have little 
use for it.

My main issue with the implementation at hand is the weird way in which 
we allow the visibility on the properties to be declared, e.g.

     public $foo { get; protected set; }

The visibility keyword on the property is watering down the visibility 
semantics. Either the property is public (then it can be set) or it is 
not public. I would have much less of a problem with this, if declaring 
visibility on a property could only be loosened but not tightened. That 
would at least be consistent with how visibility works with inheritance. 
It would be much better though to remove the visibility keyword from the 
property altogether. It almost never tells the truth.

Removing the visibility keyword from the property altogether would also 
remove the purported default visibility that is applied to the 
accessors. I say "purported" because doing

     public $foo { get; protected set; }

will also change the unset accessor to protected, which makes perfect 
sense when you think about it but is completely unobvious when the 
declaration claims the default visibility of all accessors to be public. 
The default visibility of isset is that of get and the default 
visibility of unset is that of set instead.

To add to the confusion, doing

     public $foo { get; protected set; public unset; }

WILL work and change the visibility of unset back to what the property 
claims it is. This should not be necessary when visibility on the 
keyword defines the default visibility. I guess it's safe to say that 
having the visibility on the property does very little to express 
anything meaningful about the property or the default visibility of 
*all* the accessors.

I briefly discussed possible solutions to this problem with Niki. Simply 
removing the visibility looks odd

     $foo { get; protected set; public unset; }

but might be the easiest approach. The above would follow the same rules 
visibility follows for any other methods, e.g. omitting the visibility 
on the accessor will make it public.

Both of us are not keen on introducing new keywords, though I find

     property $foo { get; protected set; public unset; }

quite nice and it is also more obvious that this is not your regular 
property. Niki suggested

     var $foo { get; protected set; public unset; }

but it's keyword recycling and one could argue that in PHP4 a var was 
always public. But then again, PHP4 didn't have the concept of 
visibility at all, so why not?

If this could be changed, I'd be more willing to vote pro the proposal.

TL;DR: The visibility keyword on the property is almost never giving an 
accurate information about the accessor visibility. Thus, it should be 
removed or replaced for clarity's sake.

Regards,
Gordon