[VOTE] Property Accessors for 5.5

https://wiki.php.net/rfc/**propertygetsetsyntax-v1.2#**votinghttps://wiki.php.net/rfc/propertygetsetsyntax-v1.2#voting

I'll say my peace on this. This is a very good implementation, and as
long as authors use accessors that depend on a separate property for
storage (like other langs require), everything will be straightforward.
Otherwise, I fear they're in for some confusing behavior.

Consider the code from the RFC:

class TimePeriod {
public $Hours {
get { return $this->Hours ?: "not specified"; }
set { $this->Hours = $value; }
}
}

$tp = new TimePeriod();
$tp->Hours; // "not specified"
isset($tp->Hours); // true!?

$tp->Hours isset, the property exists and it's value is non-null.

The auto implementation of isset compares $this->Hours to NULL, but since

$this->Hours goes through the getter, it will return "not specified". So
the property will always appear to be isset.

• The guards seem spooky: A set of tokens ($this->prop) will have varying
  behavior (e.g. direct prop read vs. getter call) depending on the call
  stack.

This is the same as would occur with isset against an undefined property,
that would call __isset(), followed by __get() which would then compare the
value to NULL.

• Giving issetter/unsetter no direct access to the property limits
  functionality and leads to weirdness like the example above.

This is possible, simply by supplying your own implementation of
isset/unset that calls isset/unset, such as:

public $foo {
get; set;
isset { return isset($this->foo); }
unset { unset($this->foo); }
}

The above five lines of code is exactly equivalent in functionality to:

public $foo;

-Clint

--

I'm not sure if this has already come up in past discussion or not. So
I apologize in advance if this is repetitive. The discussion for property
accessors has been so lengthy I couldn't find the time to keep up with it
all.

However, I found some time to play around with the patch today and I
noticed something that might use improving. When you use var_dump() on the
object properties that have defined get accessors don't produce any useful
output with var_dump. Is this fixable?

Am 22.01.2013 02:46, schrieb Clint Priest:

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

@Lester seems pretty much opt-in to me :)

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

2013/1/22 Lester Caine lester@lsces.co.uk

Clint Priest wrote:

There seems to be a lot of userland support for this proposal from people
who
don't have voting rights.

And what about the userland people who don't want the additional
complexity who don't have voting rights?

Don't use it. Regarding your other mails you are stuck with 5.2 anyway ;)

--
Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=**contacthttp://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.**ukhttp://rainbowdigitalmedia.co.uk

--

--
github.com/KingCrunch

For those that have voted against this proposal, are there any
clarifications that can be made or questions answered?
I can say that this bit is particularly confusing:

 public $foo {}

My understanding that this code will not emit any warnings when
interpreted, but the $foo property has no getter nor setter. You can
call isset which will return false but unset will emit a warning, yes?
This is correct except in the case where you are inheriting the
property. The above, on it's own class would be pointless.
I would think that the above snippet would create a property with all
public, default implementations.
If you are looking to create a public property with default
implementations, the following will suffice and has for a long time:

public $foo;

I see why it doesn't: it doesn't
permit you to declare only a getter and no setter.
Not sure what you mean here, you can declare a property with a getter
and no setter, such as:

public $foo { get; }

Which allows the property to be read, but not written, this is what the
asymmetrical access is about.

This is just one more reason for me to vote no on this.

--
-Clint

The simple explanation from me is that the ROI isn't there on this one.
It adds a lot of code complexity for very little return. Yes, it saves a
couple of lines of boilerplate code for a few people, but the cost is
high in terms of ongoing maintenance and potential issues for opcode
caches as well. If you look at the split in voting you will notice it is
pretty much split along the lines of the people who have to maintain
this code vs. the people who would like a shiny new feature.

I did a bit of testing with APC today and here is what I came up with:
https://gist.github.com/4597660 It adds the logic for copying the accessor
functions in my_copy_property_info*. This seemed to do the trick in my
testing, but I don't really know APC, so I can't judge whether it might
require further changes.

Thanks,
Nikita

For those that have voted against this proposal, are there any
clarifications that can be made or questions answered?

There seems to be a lot of userland support for this proposal from
people who don't have voting rights.
The simple explanation from me is that the ROI isn't there on this one.
It adds a lot of code complexity for very little return. Yes, it saves a
couple of lines of boilerplate code for a few people, but the cost is
high in terms of ongoing maintenance and potential issues for opcode
caches as well. If you look at the split in voting you will notice it is
pretty much split along the lines of the people who have to maintain
this code vs. the people who would like a shiny new feature.
Hey Rasmus,

With all due respect, I think you missed the post where Nikita Popov
analysed two of the biggest PHP based frameworks around, Symfony
(Standard) and Zend Framework where he found that there were 3,805 and
4,080 functions which served as a getter or a setter. I do not think
his analysis even included any usage of __get() or __set(). Here is his
original gist of his analysis:

https://gist.github.com/3884203

In terms of cost of maintenance, I was under the impression that since I
wrote it, I would be maintaining it which is why I applied for and you
approved a VCS account for me.

Lastly, I'm not sure if you have looked over the code, but a very large
amount of duplicate code has been cleaned up and centralized, both
property accessors and magic accessors use the same pre/post function
and logic. I can honestly say that adding the feature to the project
would improve the code base.

-Rasmus

--
-Clint

Rasmus,

If you look at the split in voting you will notice it is

pretty much split along the lines of the people who have to maintain
this code vs. the people who would like a shiny new feature.

I pulled some numbers, and this isn't really the case.

Based off of commits from the past year (since 2012-01-01), here are the
raw numbers:
No: 1238
Yes: 895

But if you remove the top 2 committing voters (in general, not tied to a
list), the numbers even out quite significantly:
No: 514
Yes: 456

So I think that point falls somewhat flat...

I'm not saying this to raise issues or anything. I just want to point out
that the active maintainers are working on both sides of the fence. It's
not a "maintainers" vs "the others" vote split...

Anthony

hi Rasmus,

The simple explanation from me is that the ROI isn't there on this one.
It adds a lot of code complexity for very little return. Yes, it saves a
couple of lines of boilerplate code for a few people, but the cost is
high in terms of ongoing maintenance and potential issues for opcode
caches as well. If you look at the split in voting you will notice it is
pretty much split along the lines of the people who have to maintain
this code vs. the people who would like a shiny new feature.

With all respects, this has nothing to do with a shiny new feature.

The opcode cache support is actually a problem, I'd to agree. While we
(my team and I) do more maintenance around the opcode area with APC
(read: not the opcode related area per itself, not enough time sadly),
I cannot buy your argument. This feature has been requested since
years, by many major projects, same for other long awaited features
like annotations. We simply can't keep ignoring these requests, that's
what kill us.

About opcode cache complexity, I think apc per se is full of things we
should simplify or drop as features to make the code base much smaller
and much easier to test and valid, we have discussed that already and
we disagreed. But this is a topic I really want to bring on the table
before we even consider bringing it in core (see other requests about
that).

Cheers,

Pierre

@pierrejoye

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

For those that have voted against this proposal, are there any
clarifications that can be made or questions answered?

There seems to be a lot of userland support for this proposal from
people who don't have voting rights.

The simple explanation from me is that the ROI isn't there on this one.
It adds a lot of code complexity for very little return. Yes, it saves a
couple of lines of boilerplate code for a few people, but the cost is
high in terms of ongoing maintenance and potential issues for opcode
caches as well. If you look at the split in voting you will notice it is
pretty much split along the lines of the people who have to maintain
this code vs. the people who would like a shiny new feature.

I think it would be important to gather the reasons for the No Votes and
ways how to address this problems.
Some from the previous discusisons are:

• Neglected APC support
• adding complexity to PHP Core
• Reference Handling
• Visibility Rules
• Fear of testing in production

And come up with solutions to those (like cutting some features, or putting
the code into 5.6 to allow for more testing)

Since several contributors helped with the development and committed
themselves for quite some time already, then if there were a chance to
address all the concerns then this feature could still move forward (maybe
at a later point)?

-Rasmus

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

For those that have voted against this proposal, are there any
clarifications that can be made or questions answered?

There seems to be a lot of userland support for this proposal from
people who don't have voting rights.

The simple explanation from me is that the ROI isn't there on this one.
It adds a lot of code complexity for very little return. Yes, it saves a
couple of lines of boilerplate code for a few people, but the cost is
high in terms of ongoing maintenance and potential issues for opcode
caches as well. If you look at the split in voting you will notice it is
pretty much split along the lines of the people who have to maintain
this code vs. the people who would like a shiny new feature.

Rasmus, could you maybe point out more specifically what issues you see
with the implementation (this also applies to all the other people who
voted "no" due to maintenance concerns)? It doesn't look like the RFC will
make it this time around, but it would be nice to know what issues there
are for next time. (If someone wants to give inline-comments on the
implementation, here's the PR: https://github.com/php/php-src/pull/263)

Thanks,
Nikita

Sherif,

Don't get me wrong, I liked it, but I also disliked the fact that it

introduces language changes that aren't easy to grasp or document. To me
this means we've borderline changed the behavior of a property (which most
PHP users currently understand to be a variable) into potential methods.
This is basically saying we can now completely change the behavior of a
property from a storage unit to a functional unit. That's a bit of a scary
thought to me, because it makes me question everything I know about
properties in PHP.

Except that everything that's proposed here is possible today with __get,
__set, __isset and __unset. So already today you can't assume that a
property is a "variable". In fact, you could build something like this
using __get, etc extremely dirty:

class Foo {
public function __get($name) {
$method = 'get' . ucfirst($name);
if (method_exists($this, $method)) {
return $this->method();
}
return $this->$name; // public properties
}
public function __set($name, $value) {
$method = 'set' . ucfirst($name);
if (method_exists($this, $method)) {
return $this->method($value);
}
return $this->$name = $value; // public properties
}
}

The difference in functionality that this provides is the ability to
custom-scope getters and setters. And it's a LOT cleaner...

Anthony