Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114779
MIME-Version: 1.0
References: <CAF+90c-W_g9Mt9yMNzBWfMEmQP287a7A6S75RyZo2fvUO2uGXQ@mail.gmail.com>
 <83a9d0ab-60b8-4939-8be9-bdd6ddeb46c5@www.fastmail.com> <CAF+90c-=y=t2o0O7MbcWjqGUK8Z_imGuw5St3n_fGcARMWZvxg@mail.gmail.com>
 <9e921668-369c-41f2-a207-856cd098a3f1@www.fastmail.com>
In-Reply-To: <9e921668-369c-41f2-a207-856cd098a3f1@www.fastmail.com>
Date: Tue, 8 Jun 2021 11:18:16 +0200
Message-ID: <CAF+90c8RKxeAp1YycR1qi8oXJVbqe9rk17pW2G=C-fBA+R4ATg@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000597ae805c43da437"
Subject: Re: [PHP-DEV] [RFC] Readonly properties
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000597ae805c43da437
Content-Type: text/plain; charset="UTF-8"

On Mon, Jun 7, 2021 at 4:09 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Mon, Jun 7, 2021, at 4:06 AM, Nikita Popov wrote:
> > On Sat, Jun 5, 2021 at 6:51 PM Larry Garfield <larry@garfieldtech.com>
> > wrote:
>
> > > Thank you for the detailed analysis in the rationale section.  I am,
> > > however, still skeptical of this approach, for a couple of reasons.
> > >
> > > 1. This does appear to address the "leakage" problem I noted in my
> earlier
> > > analysis around the start of the year, when considering the writeonce
> > > proposal[1][2].  That's great to see.
> > >
> > > 2. It doesn't address the larger question of cloning, however.  The
> answer
> > > for now seems "maybe we'll get clone-with at some point", which would
> be a
> > > way around it, but there is no active RFC for that right now.  I'm
> > > obviously very in favor of RFCs that complement each other to give more
> > > than the sum of their parts, but those RFCs need to be at least on the
> > > horizon to actually come together.  Right now that looks like it won't
> > > happen this cycle.  Absent clone-with, readonly would be effectively
> > > unusable in any evolvable object of any non-trivial complexity.
> >
> >
> > Compared to the general area of applicability of readonly properties, the
> > cases that require clone-with are rare. It's the intersection of
> > "not-really-immutable objects using wither evolution" and "has so many
> > properties that passing them to the constructor is infeasible". While
> this
> > intersection does include a couple of prominent examples, they are by no
> > means numerous. While it may be a problem worth solving, I *personally*
> > don't consider it neither particularly important nor time critical.
> >
> > For what it's worth, I believe Mate wants to work on clone-with once this
> > RFC passes -- while there is no RFC for that, there is already an
> > implementation, though from a cursory look it would require some
> > adjustments to actually work with readonly properties.
>
> I look forward to it.
>
> I don't think the use cases for non-trivial structs (where a withX()
> method can just return new static(list all properties here) ) are as few as
> you think.  One difficulty in determining that is that, if they are made
> easier/more robust (via any of the mechanisms under discussion), their
> usage is likely to increase.  That's a good thing, and a benefit of all of
> these pieces floating around, but it does make it harder to gauge the
> net-use of any particular feature.  I prefer to err on the side of enabling
> more flexibility and power rather than less.
>
> > > It also wouldn't work with objects that need properties that are not
> > > constructor arguments, such as PSR-7 type objects.  That's a no in my
> book.
> > >
> >
> > I don't understand what you mean here. Why would properties that are not
> > constructor arguments be a problem?
>
> Without clone-with, if you wanted a with-er method, it would necessarily
> look like this:
>
> class Point {
>   public function __construct(
>     public readonly int $x,
>     public readonly int $y,
>     public readonly int $z,
>   ) {}
>
>   public function withX(int $newX) {
>     return new static($newX, $this->y, $this->z);
>   }
> }
>
> That is, you're populating the entire property set via the constructor.
> In some cases that's fine; the larger the object, though, the more
> needlessly verbose redundancy that has.
>
> Now let's add a "rendered" flag property, that is public readonly bool.
> (Maybe not the best example; it's before 9 am and I'm trying to think on
> the fly here.)  It's not in the constructor, but for later processing.  It
> would get set as a side effect of some other method, to indicate the point
> has already been drawn.  Now make a new version of the object with a
> different Z.  What do you do with $rendered?
>
> > > 3. As noted in my previous analysis, even with clone-with, asymmetric
> > > visibility results in a nicer syntax when evolving objects that have
> any
> > > complexity to them.  (See the examples in [2].)
> > >
> >
> > I tend to agree. And I am not opposed to having asymmetric visibility as
> > well. For example, C# does both, and I think both do have value.
> "readonly"
> > is a very strong signal to the reader. Asymmetric visibility is not.
> > Asymmetric visibility (without proper readonly support) could mean that
> the
> > property is actually readonly, or it could mean that it's exactly what it
> > says on the tin: The property is internally mutable. I don't think these
> > two concepts should be conflated, though they certainly have overlap.
>
> I think this is the crux of the disagreement.  There's a large swath of
> cases that could be well-handled by either.  I feel the cases that are only
> or better handled by asymmetric visibility is larger than the cases that
> are only or better handled by readonly.  As noted above, I don't know that
> either of us have any data with which to determine which is actually the
> case.
>
> (Maybe someone with more C# experience can speak to that?  What's typical
> there, where both mechanisms are available?)
>
> > > 5. I would have to experiment a bit with hydration, as others have
> noted,
> > > because unconventional object construction mechanisms are a critically
> > > important workflow.  The RFC even notes in passing that serialization
> is
> > > possibly made complicated.  Just how complicated?  There's no mention
> of
> > > __deserialize() and how it would interact, but cases like that need to
> be
> > > very clearly handled and documented.
> > >
> >
> > The RFC notes in passing that *if* readonly default values were allowed,
> it
> > could have impact on userland deserialization. As-is, I believe the
> > proposal doesn't present any problem for userland hydrators or
> > deserializers, or PHP's own unserialization mechanisms. This should get
> > mentioned in the proposal, though it will be along the lines of "yes,
> > everything works as expected".
> >
> >
> > > 6. I'm OK with the approach to constructor promotion and default
> values.
> > > That seems reasonable in context, especially if combined with
> > > New-in-initializers[3], which I am hoping you plan to finish this
> cycle. :-)
>
> > 8. Although the RFC says it does not preclude accessors or explicit
> > > asymmetric visibility in the future, and I absolutely believe that
> Nikita
> > > is genuine about that, I am still concerned that should more robust
> > > proposals come forward later, it will be met with "we don't need
> another
> > > still-fancier syntax here, readonly is good enough."  It's good to know
> > > that C# manages to have both, but that doesn't mean the same logic
> would
> > > apply in PHP, or to PHP voters, specifically.
> > >
> >
> > To clarify, I think your argument here goes roughly like this (correct me
> > if I'm wrong): If we already have one of readonly properties or
> asymmetric
> > visibility, an argument could be made for not adding the other one
> because
> > it doesn't provide sufficient marginal value. So, if we assume that we
> can
> > only have one of these features, is readonly properties the right one to
> > add? Shouldn't we add asymmetric visibility instead, which can be used
> for
> > a wider range of use cases?
>
> That is essentially correct.  I know you're not making that argument, and
> I'm not making that argument, but it's an argument I can absolutely see
> being made, because it has been in the past for other things, and sometimes
> I agree with it as it can be a valid argument.
>
> Hence why, in general, I prefer to go with the more robust, more
> "interacts nicely with other things" option from the get-go, to both not
> make more robust options harder to adopt in the future and to avoid user
> confusion down the road when they have 3 different tools that all do
> *mostly* the same thing and aren't sure of the subtle reasons why they'd
> use one over the other.
>
> (Like, for example, if one had a readonly property but wanted to convert
> it to a custom accessor property... what are the subtle knock on effects
> they have to account for, then?  There are almost certainly more of them
> than if switching a property from implicit accessors to explicit accessors,
> although off hand I don't know what they would all be.)
>
> > I think this is a legitimate concern to have, but (under the assumption
> > that we can have only one) it's not easy to say which would be
> preferable.
> > Yes, asymmetric visibility is more widely applicable, but at the same
> time
> > I would expect most applications to be bonafide readonly properties. So
> > while asymmetric visibility is more widely applicable, it's also
> imprecise
> > for the majority use case. Which is better? Of course, my assumption here
> > could also be wrong.
>
> As above, I'd love to get some actual data from the C# community on this.
> Lacking that, we're all just guessing.>


Well, I think we can get an approximation like this:

https://beta.grep.app/search?q=%7B%20get%3B%20private%20set%3B%20%7D&filter[lang][0]=C%23
{ get; private set; } 74,281 results
https://beta.grep.app/search?q=%7B%20get%3B%20%7D&filter[lang][0]=C%23 {
get; } 156,304 results
https://beta.grep.app/search?q=readonly&filter[lang][0]=C%23 readonly
409,585 results

Note that the "{ get; }" part includes both readonly properties and
get-only abstract properties, so that one is an overestimate. Using
variants like \{\s+get;\s+private set;\s+\} doesn't change the results
materially.

Unless my methodology is completely borked, the ratio of readonly to
asymmetric visibility seems to be something like 4:1 even as a conservative
estimate.

Regards,
Nikita

--000000000000597ae805c43da437--