On 11.07.25 11:38, Nick wrote:
On 11. Jul 2025, at 14:25, Claude Pache <claude.pa...@gmail.com> wrote:Le 11 juil. 2025 à 06:30, Nick <p...@nicksdot.dev> a écrit : To not get this buried in individual answers to others:I came up with two alternative implementations which cache the computed `get` hook value. One leverages separate cache properties, the other writes directly to the backing store.Links to the alternative branches can be found in the description of the original PR.https://github.com/php/php-src/pull/18757I believe that these are fair solutions to address the concerns that came up in the discussion, and I hope people will agree.*Cheers,* NickHi Nick, I think that the second alternative described as: “Cache computed get hook to it's backing store; never run the hook again”is near the most reasonable from my point of view (basing my judgment on the description, as I have not looked the actual implementation), although there are still some concerns.Advantages of that approach:1. relatively to the first alternative solution: there is indeed no point to have a cache separate from the backing-store, as the backing-store is supposed to play that role in most cases;2. relatively to the manual “??=” pattern, it works correctly with nullable properties.Here are my remaining concerns: A. It may be confusing to have a getter that is not always called.B. The idea of returning the value directly from the backing-store if initialised is useful also for non-readonly properties. (That can be emulated with the “??=” pattern, but only if the property is not nullable.)Both concerns may be resolved with the following amendment:* Introduce a `cached` modifier, that enables the caching semantics (i.e., not executing the getter if the backing-store is initialised).The example from the RFC would be written as: ```php readonly class LazyProduct extends Product { private DbConnection $dbApi; private string $categoryId; public Category $category { cached get => $this->dbApi->loadCategory($this->categoryId); } } ```The `cached` modifier may be applied to any get hook, but it is mandatory if the property is readonly.(In practice, the “cached get hook” corresponds to my originally proposed “init hook”, but with the advantage of not having a separate hook.)—ClaudeHey Claude, I agree, the second alternative is more neat.A.Three people seem to think that always running the hook when returning a cached value would cause confusion. You are now arguing for the exact opposite. I don’t have a very strong opinion here. However, I also came to the conclusion that caching and not running the hook is the more obvious and less confusing approach.Also, naturally it’s more performant to not run the hook again. Running the hook again, but then not updating the value feels off.And updating the value would mean no caching, which brings us to the very beginning and your very concern.So, yeah… that’s that. :)B.I am afraid here I do have a strong opinion. Please remember my very first mail before discussion [1]. While for Larry the main reason for `readonly` hooks is lazy-initialisation, for me it is to write less code, to have less noisy classes.I don’t want to be forced to add readonly to each property.Now you are proposing a mandatory `cached` modifier. Which means *checks notes*, I would save 2 characters on each property. You will understand that this is not in my interest, and not what I am proposing here.But aside from that I do not like to write more code. I honestly don’t see the benefit of having this modifier in the first place. How “alternative implementation 2” works now is IMO just good. And it makes sense because it is limited to the `readonly` context. There is no technical need for the modifier. As we see, because we have a working solution at hand.For me: It is clear, it’s solving the main issue you rightfully brought up, and it is less code to write.I can imagine that `cached` could be helpful on non `readonly` properties in some cases. However, I feel that brings us almost in similar territory than immutable classes. It is a whole new topic which I believe really shouldn’t be part of this RFC that explicitly . Can we please agree on that it is future scope whether or not non `readonly` hooks should get such a modifier?I’d appreciate if we could settle with “alternative implementation 2” as proposed.
Hi Nick, Claude,I think it's important to explicitly mark it as "this value will be stored in memory", I mean just silently caching the get hook could quickly lead to unexpected behavior. Like one would expect the value to be changed and another one wonders why a big chunk of memory will not be freed `get => $this->readBigFile();`.
On the one hand I like the cached modifier but personally I would prefer a separate init hook because it seems to be more clear that this is a backed property that will be initialized once.
The cached modifier I would expect to be an attribute applicable to any function which uses another cache store similar to how it's possible in python to memorize function calls which would be a very different feature.
Just my two cents from someone without voting rights
*Cheers,* Nick [1] https://externals.io/message/127529
OpenPGP_0x3936ABF753BC88CE.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
