On Mon, Jun 7, 2021, at 4:06 AM, Nikita Popov wrote:
> On Sat, Jun 5, 2021 at 6:51 PM Larry Garfield <la...@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.
> 9. I know that the variance for properties in child classes was a source of
> > discussion, and the end result seems to be that readonly-ness is
> > invariant. However, that precludes having an easy way to have both a
> > mutable and immutable version of a class, easily. (If you wanted to have,
> > say, a read-only class most of the time for safety, but for writing you use
> > an alternate "open" version that can be updated and then persisted to the
> > database.) That's a style I've experimented with on and off for a while
> > and already don't have a great solution to, but it feels like readonly
> > would make that even harder. Again, I'd be very happy to hear alternatives
> > around that.
> >
>
> I don't believe you can't have an immutable class extend a mutable one or a
> mutable extend an immutable one, both result in LSP violations (though not
> on the type level). There was a lot of discussion around this when
> DateTimeImmutable was introduced, which notably is not in an inheritance
> relationship (in other direction) with DateTime, for good reason.
True. Might that be feasible with asymmetric visibility, however? (I'm
honestly not sure.)
> > Depending on the answers to the above, I could be convinced of this as a
> > stop gap iff paired with clone-with in the same version. However, on its
> > own I think this is only a half-solution, and I'm not wild about a
> > half-solution for a version or two. That's why I prefer going straight to
> > asymmetric visibility, as that would cover mostly the same use case in a
> > single RFC while still being forward compatible; it would benefit from
> > clone-with, but still be usable without it.
> >
>
> Just to reiterate this point: Readonly properties is not a half solution to
> which asymmetric visibility is the full solution. They are *different*
> solutions to different albeit overlapping problems. I'm not proposing
> readonly properties here because it's the only thing I can get into 8.1,
> I'm proposing it because I think it's the more important problem to solve.
I was perhaps unclear here. I don't think readonly is a half-solution and
asymmetric visibility is the full solution. I think readonly without
clone-with is a half-solution. readonly plus clone-with is a mostly-full
solution that ends up in a very similar (competing?) place with asymmetric
visibility, which is a mostly-full solution. I can probably still vote for
readonly plus clone-with, because that combination would be useful. It's
readonly without clone-with that I am concerned about being a half-solution.
(clone-with is a nice-to-have with asymmetric visibility, but IMO is an
ergonomic necessity with readonly.)
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
