On Mon, Jun 7, 2021 at 4:09 PM Larry Garfield <la...@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 <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.> 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