[VOTE] Readonly properties

4 years ago by Hamza Ahmad — view source — reply

unread

Hi Nikita,

It is going to be the second contribution from you regarding OOP.
However, this proposal, as you have mentioned, has some issues with
cloning. What if you would have opened voting after fixing this?

Or, your mood is to get it passed and then fix it before final release of 8.1?

Thank you

Hamza Ahmad

Hi internals,

I have opened voting on https://wiki.php.net/rfc/readonly_properties_v2.
The vote closes 2021-07-15.

See https://externals.io/message/114729 for the discussion thread on this
proposal. I think a decent tl;dr is that readonly properties as proposed do
not play well with clone-based withers, so some people believe we should
either improve cloning first, or introduce asymmetric property visibility
instead, which does not suffer from this issue.

Regards,
Nikita

4 years ago by Nicolas Grekas — view source — reply

unread

Le jeu. 1 juil. 2021 à 12:23, Nikita Popov nikita.ppv@gmail.com a écrit :

Hi internals,

I have opened voting on https://wiki.php.net/rfc/readonly_properties_v2.
The vote closes 2021-07-15.

See https://externals.io/message/114729 for the discussion thread on this
proposal. I think a decent tl;dr is that readonly properties as proposed do
not play well with clone-based withers, so some people believe we should
either improve cloning first, or introduce asymmetric property visibility
instead, which does not suffer from this issue.

Hi NIkita,

I voted against the proposal because it doesn't work with cloning at all.

Cloning is a critical feature of stateful objects, and we should solve it
the same version that introduces readonly IMHO.

If we figure out that we can't agree on a sensible improved behavior for
cloning, we're going to be in a dead-end with readonly.

I think we are not in a hurry and that we should wait for the RFC that
improves cloning to add readonly.

In another thread, you write:

It's okay to vote against this if cloning is a deal breaker. In that case

I'll probably either work on cloning before re-proposing this, or pivot to
asymmetric visibility -- it's not my first preference, but it may be the
more pragmatic choice. Cloning is definitely the weak point of this
proposal.

I think this is a strong enough weak point to warrant postponing the
decision. Also, this very statement about asymmetric visibility being more
pragmatic is a string hint that we need time to explore it at the same time
IMHO.

Cheers,
Nicolas

4 years ago by Pierre — view source — reply

unread

Le 01/07/2021 à 16:38, Nicolas Grekas a écrit :

Hi NIkita,

I voted against the proposal because it doesn't work with cloning at all.

Cloning is a critical feature of stateful objects, and we should solve it
the same version that introduces readonly IMHO.

If we figure out that we can't agree on a sensible improved behavior for
cloning, we're going to be in a dead-end with readonly.

I respectfully disagree.

Having readonly properties and immutable objects is a must have, but
changing property of an object while cloning, not so much. There's many
case where readonly properties will be valuable where you never need to
clone your objects. Actually, cloning objects is not something you do
every day.

Please note that I agree with you that advanced / flexible clone
semantics would be a nice to have, but I don't see the lack of it
blocking for readonly properties.

I personally don't have any real use case where I couldn't implement
withers on my objects doing the same than dedicated advanced clone
semantics. Could you please provide some real world examples ? People
could change their minds if they could see why it's so blocking for you.

Regards,

--

Pierre

4 years ago by Larry Garfield — view source — reply

unread

Le 01/07/2021 à 16:38, Nicolas Grekas a écrit :

Hi NIkita,

I voted against the proposal because it doesn't work with cloning at all.

Cloning is a critical feature of stateful objects, and we should solve it
the same version that introduces readonly IMHO.

If we figure out that we can't agree on a sensible improved behavior for
cloning, we're going to be in a dead-end with readonly.

I respectfully disagree.

Having readonly properties and immutable objects is a must have, but
changing property of an object while cloning, not so much. There's many
case where readonly properties will be valuable where you never need to
clone your objects. Actually, cloning objects is not something you do
every day.

Please note that I agree with you that advanced / flexible clone
semantics would be a nice to have, but I don't see the lack of it
blocking for readonly properties.

I personally don't have any real use case where I couldn't implement
withers on my objects doing the same than dedicated advanced clone
semantics. Could you please provide some real world examples ? People
could change their minds if they could see why it's so blocking for you.

Regards,

The most famous use case right now for with-er objects is PSR-7, which is where the naming convention comes from. I cannot say how widely used it is outside of FIG-inspired value objects, but I am pretty sure it is used.

The key point is that you rarely need to clone service objects; value objects, however, you have to clone if you want to mutate. Look at any PSR-7 pipleline. By design, it calls $request->withBlah($newBlah) a lot, and returns a new object. That's the model that we want to support, and make easier to do, but the readonly flag makes harder to do than the status quo today. (See my previous post on the subject in the last thread.)

There are use cases for readonly that don't require cloning. For those, it's useful. I personally think asymmetric visibility would render readonly unnecessary and redundant, but Nikita disagrees, and he's the one writing the code so... :-)

The best case scenario is by 8.2 we end up with asymmetric visibility and clone-with, and combined with readonly we get a huge array of options for how to lock down value objects and still make them evolvable. The worst case scenario is we find that readonly cannot be extended to support clone-with for some hand-wavy engine reasons, at which point it becomes largely vestigial in favor of asymmetric visibility and clone-with.

--Larry Garfield

4 years ago by Deleu — view source — reply

unread

On Thu, Jul 1, 2021 at 5:25 PM Larry Garfield larry@garfieldtech.com
wrote:

Le 01/07/2021 à 16:38, Nicolas Grekas a écrit :

Hi NIkita,

I voted against the proposal because it doesn't work with cloning at
all.

Cloning is a critical feature of stateful objects, and we should solve
it
the same version that introduces readonly IMHO.

If we figure out that we can't agree on a sensible improved behavior
for
cloning, we're going to be in a dead-end with readonly.

I respectfully disagree.

Having readonly properties and immutable objects is a must have, but
changing property of an object while cloning, not so much. There's many
case where readonly properties will be valuable where you never need to
clone your objects. Actually, cloning objects is not something you do
every day.

Please note that I agree with you that advanced / flexible clone
semantics would be a nice to have, but I don't see the lack of it
blocking for readonly properties.

I personally don't have any real use case where I couldn't implement
withers on my objects doing the same than dedicated advanced clone
semantics. Could you please provide some real world examples ? People
could change their minds if they could see why it's so blocking for you.

Regards,

The most famous use case right now for with-er objects is PSR-7, which is
where the naming convention comes from. I cannot say how widely used it is
outside of FIG-inspired value objects, but I am pretty sure it is used.

The key point is that you rarely need to clone service objects; value
objects, however, you have to clone if you want to mutate. Look at any
PSR-7 pipleline. By design, it calls $request->withBlah($newBlah) a lot,
and returns a new object. That's the model that we want to support, and
make easier to do, but the readonly flag makes harder to do than the
status quo today. (See my previous post on the subject in the last thread.)

There are use cases for readonly that don't require cloning. For those,
it's useful. I personally think asymmetric visibility would render
readonly unnecessary and redundant, but Nikita disagrees, and he's the one
writing the code so... :-)

The best case scenario is by 8.2 we end up with asymmetric visibility and
clone-with, and combined with readonly we get a huge array of options for
how to lock down value objects and still make them evolvable. The worst
case scenario is we find that readonly cannot be extended to support
clone-with for some hand-wavy engine reasons, at which point it becomes
largely vestigial in favor of asymmetric visibility and clone-with.

--Larry Garfield

--

To unsubscribe, visit: https://www.php.net/unsub.php

I'd say don't use readonly on PSR-7. I have so many use cases for readonly
property and no use case for cloning. readonly syntax is far superior than
asymmetric visibility and will be almost as good as Constructor Promotion
Property. I would even go as far to say that I don't need anything more
than just readonly as-is. I think the bigger picture here is how many use
cases are there that would vastly benefit from this Vs how many use cases
could potentially benefit from it but won't because of lack of cloning
support. Of course everyone's opinion will be shaped by the universe they
live in and in mine this RFC covers everything I need with no drawbacks and
I honestly don't understand not wanting this just because of lack of
cloning.

--
Marco Aurélio Deleu

4 years ago by Marco Pivetta — view source — reply

unread

I honestly don't understand not wanting this just because of lack of
cloning.

Agreed: improvements on cloning ergonomics can come later.

Also, the problem goes away the smaller your state becomes: this may
encourage some design towards more granular data-structures.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

4 years ago by Pierre — view source — reply

unread

Le 01/07/2021 à 17:25, Larry Garfield a écrit :

The most famous use case right now for with-er objects is PSR-7, which is where the naming convention comes from. I cannot say how widely used it is outside of FIG-inspired value objects, but I am pretty sure it is used.
As long as you implement the with-ers manually, you don't need "clone
with", you can create a new instance the old way (i.e. using the object
constructor). Once there will be a "clone with" like feature, PSR's will
be able to evolve, but right now they'll work as they are, and it's fine.
The key point is that you rarely need to clone service objects; value objects, however, you have to clone if you want to mutate. Look at any PSR-7 pipleline. By design, it calls $request->withBlah($newBlah) a lot, and returns a new object. That's the model that we want to support, and make easier to do, but the readonly flag makes harder to do than the status quo today. (See my previous post on the subject in the last thread.)

That's the whole point of readonly properties, not to mutate. If you
want readonly mutable properties, don't write the readonly keyword. Any
attempt in mutating something that was explicitly closed sounds like
you're doing something really wrong.

I mean by that that not all objects are services, value objects are
values, they're not meant to mutate. If you need a different value, you
use the object constructor. The right way in my opinion for PSR-7
pipeline is to stick with the with-ers, because it makes it explicit, by
contract, of what you are attempting to achieve in your middlewares, and
yet still work using an interface and not an instance: this model is
very flexible and highly extensible. And yes PSR-7 doesn't fit with
readonly properties, but that's not a problem, having readonly
properties doesn't mean you actually have to use them everywhere.

There are use cases for readonly that don't require cloning. For those, it's useful. I personally think asymmetric visibility would render readonly unnecessary and redundant, but Nikita disagrees, and he's the one writing the code so... :-)

Yeah, I very much like asymmetric visibility as well. But readonly and
asymmetric visibility are not incompatible. They both answer to
different needs, some of those use cases probably intersect, but
readonly properties have the nice advantage of allowing you to write
pure immutable objects with a very simple syntax, and I love that,
there's much code I wrote and do maintain that'll fit perfectly with that.

Regards,

--

Pierre

4 years ago by me@jhdxr.com — view source — reply

unread

Hi Nikita,

Thanks for the RFC. A quick question, is it allowed to assign to a readonly property multiple times within the construction method? Sorry for it if someone has asked this before, because actually I didn't go through the whole thread, only did a quick search on it. Thanks.

Regards,
CHU Zhaowei

4 years ago by Ilija Tovilo — view source — reply

unread

Hi

Thanks for the RFC. A quick question, is it allowed to assign to a readonly property multiple times within the construction method? Sorry for it if someone has asked this before, because actually I didn't go through the whole thread, only did a quick search on it. Thanks.

No, assignment is exclusively allowed under two conditions:

The property is declared in the same class
The property is uninitialized

This means you won't be able to change a property once you've written
a value to it, even in the constructor.

Ilija

4 years ago by Patrick ALLAERT — view source — reply

unread

Le jeu. 1 juil. 2021 à 12:23, Nikita Popov nikita.ppv@gmail.com a écrit :

Hi internals,

I have opened voting on https://wiki.php.net/rfc/readonly_properties_v2.
The vote closes 2021-07-15.

See https://externals.io/message/114729 for the discussion thread on this
proposal. I think a decent tl;dr is that readonly properties as proposed do
not play well with clone-based withers, so some people believe we should
either improve cloning first, or introduce asymmetric property visibility
instead, which does not suffer from this issue.

Regards,
Nikita

Hi Nikita,

As much as I wish I could access publicly a property, but being denied
changing it:

var_dump($object->property); // allowed
$object->property = $newObject; //disallowed

I feel that the "readonly" feature will be used instead of asymmetric
visibility as suggested in "property accessors" RFC in cases where the
latter would be more appropriate.

I wish "Property accessors" RFC would have been open to vote for 8.1 as you
probably initially intended to for the following reasons:

It doesn't suffer from the clone-based withers issue.
It provides a way to declare properties in interfaces.

Maybe there are other drawbacks with that one that I don't see? Probably
the much higher complexity of it?

Although they are different concepts, I'm not sure we would benefit much
from "readonly" if we already had property accessors.

Just wanted to express my preferences.

Thanks for all the work!

Regards,
Patrick

4 years ago by Patrick ALLAERT — view source — reply

unread

Le jeu. 1 juil. 2021 à 12:23, Nikita Popov nikita.ppv@gmail.com a écrit :

Hi internals,

I have opened voting on https://wiki.php.net/rfc/readonly_properties_v2.
The vote closes 2021-07-15.

See https://externals.io/message/114729 for the discussion thread on this
proposal. I think a decent tl;dr is that readonly properties as proposed do
not play well with clone-based withers, so some people believe we should
either improve cloning first, or introduce asymmetric property visibility
instead, which does not suffer from this issue.

Regards,
Nikita

With the proposed implementation the following class:

class A {
public readonly string $name;

public function __construct(string $name) {
    $this->setName($name);
}

public function setName(string $name) {
    $this->name = $name;
}

public function setName2(string $name) {
    $this->setName($name);
}

public function setName3(string $name) {
    $this->name = $name;
}

}

Would behave like this:

$a = new A("Initial name");
echo $a->name, "\n"; // echoes Initial name

$a->setName("New name"); // Allowed ?
echo $a->name, "\n"; // echoes New name

$a->setName2("Yet another name"); // Allowed ?
echo $a->name, "\n"; // echoes Yet another name

$a->setName3("Yet another name"); // Not allowed, although identical to
setName()?

I find the word "scope" very confusing to understand in:

A readonly property can only be initialized once, and only from the scope
where it has been declared. Any other assignment or modification of the
property will result in an Error exception.

Is the above behaviour an implementation bug or does the RFC implies that?

Regards,
Patrick

4 years ago by Nikita Popov — view source — reply

unread

On Mon, Jul 5, 2021 at 1:56 PM Patrick ALLAERT patrickallaert@php.net
wrote:

Le jeu. 1 juil. 2021 à 12:23, Nikita Popov nikita.ppv@gmail.com a
écrit :

Hi internals,

I have opened voting on https://wiki.php.net/rfc/readonly_properties_v2.
The vote closes 2021-07-15.

See https://externals.io/message/114729 for the discussion thread on this
proposal. I think a decent tl;dr is that readonly properties as proposed
do
not play well with clone-based withers, so some people believe we should
either improve cloning first, or introduce asymmetric property visibility
instead, which does not suffer from this issue.

Regards,
Nikita

With the proposed implementation the following class:

class A {
public readonly string $name;
public function __construct(string $name) {
    $this->setName($name);
}

public function setName(string $name) {
    $this->name = $name;
}

public function setName2(string $name) {
    $this->setName($name);
}

public function setName3(string $name) {
    $this->name = $name;
}
}

Would behave like this:

$a = new A("Initial name");
echo $a->name, "\n"; // echoes Initial name

$a->setName("New name"); // Allowed ?
echo $a->name, "\n"; // echoes New name

$a->setName2("Yet another name"); // Allowed ?
echo $a->name, "\n"; // echoes Yet another name

$a->setName3("Yet another name"); // Not allowed, although identical to
setName()?

[...]

Is the above behaviour an implementation bug or does the RFC implies that?

This is indeed an implementation bug. Thanks for catching it! The issue
here is that the property write gets cached and bypasses the readonly
check, which of course shouldn't be happening.

Regards,
Nikita

4 years ago by Nikita Popov — view source — reply

unread

Hi internals,

I have opened voting on https://wiki.php.net/rfc/readonly_properties_v2.
The vote closes 2021-07-15.

Readonly properties have been accepted with 38 votes in favor and 11
against.

Regards,
Nikita