Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122474
Message-ID: <a4c62d60-63af-45f0-843b-a39646b7bbed@bastelstu.be>
Date: Thu, 22 Feb 2024 20:53:17 +0100
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Content-Language: en-US
To: Larry Garfield <larry@garfieldtech.com>,
 php internals <internals@lists.php.net>
References: <fca97d6c-d4ec-4000-958a-72c32beb8421@app.fastmail.com>
In-Reply-To: <fca97d6c-d4ec-4000-958a-72c32beb8421@app.fastmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=C3=BCsterhus?=)

Hi

1. I believe there is an error in the "Final Hooks" section:

 > // But this is NOT allowed, because beforeSet is final in the parent.

I believe it should be s/beforeSet/set/.

2. In the same section this sentence is probably grammatically incorrect.

 > Declaring hooks final on a property that is declared final is 
redundant will throw an error.

3. Regarding the same sentence I also don't see why that should be 
disallowed, even if it is redundant. It's also legal to define 'final' 
functions within a 'final' class. Which also brings me to the question: 
Is defining 'final' properties and 'final' hooks within a 'final' class 
equally disallowed?

4. Since the RFC was last discussed, the #[\Override] RFC was accepted 
and implemented (https://wiki.php.net/rfc/marking_overriden_methods). A 
short section on the interaction with #[\Override] would probably be 
helpful.

5. Not sure if I've asked it before, but have you considered making the 
parameter for ReflectionProperty::getHook() an enum?

On 2/21/24 19:55, Larry Garfield wrote:
> There’s one outstanding question, which is slightly painful to ask: Originally, this RFC was called “property accessors,” which is the terminology used by most languages.  During early development, when we had 4 accessors like Swift, we changed the name to “hooks” to better indicate that one was “hooking into” the property lifecycle.  However, later refinement brought it back down to 2 operations, get and set.  That makes the “hooks” name less applicable, and inconsistent with what other languages call it.
> 
> However, changing it back at this point would be a non-small amount of grunt work. There would be no functional changes from doing so, but it’s lots of renaming things both in the PR and the RFC. We are willing to do so if the consensus is that it would be beneficial, but want to ask before putting in the effort.
> 

Calling it hooks is fine and allows for future extension without 
renaming, should the need arise.

Best regards
Tim Düsterhus