Re: [VOTE] Property Accessors for 5.5

From: Date: Tue, 22 Jan 2013 16:20:28 +0000
Subject: Re: [VOTE] Property Accessors for 5.5
References: 1 2 3  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message 
Am 22.01.2013 02:46, schrieb Clint Priest:


On 1/20/2013 3:11 PM, Gordon Oheim wrote:

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.

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.


This is basically an argument against asymmetrical visibility altogether
which is one of the most useful features of this proposal.  I think
there are many use cases where an object wants to be able to emit a
message (value) but not receive one and being able to declare that fact
is important.



I understand the purpose and utility of having different levels of visibility. I am not argueing against that. I am argueing that putting the visibility on the property does not give accurate information about the property's visibility itself.

Contrived Example: doing just public $foo (Traditional Property) means I can access the property from everywhere directly. But doing public $foo {} (Guarded Property) means I cannot access the property *at all*. So we went through full access to no access just by adding curly braces. Likewise, private $foo { public get; public set; } is not private at all.

When the visiblity only applies to declared accessors we should just declare in on the accessors only. That makes for better semantics. After all, it's a guarded property, which already implies that it is hidden behind accessors and thus non-public.


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.


Perhaps this is just a documentation problem in that somewhere the
implication that public applies to all accessors when it should really
say that it applies to all declared accessors.


Yes, that should be stretched. I think the final documentation should also strongly emphasize that Guarded Properties is an entirely new concept and not some extension to Traditional Properties or to Magic Methods. We should prevent people from trying to map these new Guarded Properties onto their existing mental models of how Traditional Properties or Magic Methods work.



It is perfectly logical that when you cannot set a value, then you
cannot unset a value.

Defining what you have done above is declaring that you want to allow
people to get or unset the value, but not set it directly. This makes
sense in some usage scenarios.  As an example, perhaps $foo represents
an internally generated id which is generated on the first call to the
getter, you don't want outsiders to specify the id, but you're okay with
having it unset so that a new one can be generated with the internal
generation mechanics on the next get request.

As an aside, your previous example can also be declared like this:

     public $foo { get; protected set; unset; }


unset is explicitly declared and implicitly implemented and is public,
isset would be implicitly declared and implicitly implemented and follow
the visibility of the getter.

isset/unset only follow the get/set visibility if isset/unset are left
undeclared.


I agree that set and unset would usually have the same visibility, but I do not agree that this should happen magically. Implicit means hidden and hidden always leads to WTF moments. I understand the convenience of having this happen automatically when this is how you'd use it most of the time. But it's still a side-effect and your UseCase shows there are exceptions where you would not want to have it that way. Maybe it should just not happen silently?



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.


I think I've made the case above that this isn't accurate, it always
affects declared accessors which do not have their own visibility
specified.



Thanks for your clarification. I'd still favor dropping the visibility from the property though. Your summary of what the visibility does is giving a clear description of the problem I see: the visibility is not about the property at all (like it is with traditional properties) and it only affects a subset of accessors (declared ones).



Regards,
Gordon

Thread (51 messages)

« previous php.internals (#65089) next »