Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122479
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
Mime-Version: 1.0 (1.0)
Subject: Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Message-ID: <6F46A2EB-E77C-41AB-B27D-88C385381E39@koalephant.com>
Date: Fri, 23 Feb 2024 23:57:19 +0700
To: php internals <internals@lists.php.net>
From: php-lists@koalephant.com (Stephen Reay)

=EF=BB=BF
> On 23 Feb 2024, at 22:58, Larry Garfield <larry@garfieldtech.com> wrote:
>=20
> On Fri, Feb 23, 2024, at 8:33 AM, Stephen Reay wrote:
>>> On 23 Feb 2024, at 06:56, Larry Garfield <larry@garfieldtech.com> wrote:=

>=20
>> Hi Larry,
>> It's good to see this idea still progressing.
>> I have to agree with the other comment(s) that the implicit
>> `$field`/`$value` variables seem odd to me. I understand the desire for
>> brevity and the potential future scope of reused hooks, but this
>> concept seems to fly in the face of many years of PHP reducing "magic"
>> like this.
>=20
> The "magic" that PHP has been removing is mostly weird and illogical type c=
asting.  As noted, neither of these variables are any more "magic" than $thi=
s.
>=20
> However, since it seems no one likes $field, we have removed it from the R=
FC.  Of note, to respond to your comment further down, $this->{__PROPERTY__}=
 will not work.  The virtual-property detection looks for the AST representa=
tion of $this->propName, and only that.  Dynamic versions that turn into tha=
t at runtime cannot work, as it needs to be known at compile time.

I guess it's mostly irrelevant on single-use hooks anyway, but that sounds l=
ike a potential gotcha with reusable hooks, and I think it's worth making it=
 very clear *now* in the RFC/docs that dynamic access like this won't work a=
s expected (and why). Perhaps some other indicator on reusablele hooks can b=
e used at that point, to signify if it's virtual or not.


> For $value, however, we feel strongly that having the default there is a n=
ecessary part of the ergonomic picture.  In particular, given your comments h=
ere:
>=20
>> To give one answer to your question about ambiguity if the `$value`
>> parameter is required - I don't believe this is actually ambiguous, in
>> the context of PHP:
>> - method parameters in child classes don't implicitly 'inherit' the
>> parent method parameter's type if they don't define one (they widen to
>> mixed);
>> - method return types have no implicit inheritance, they must declare a
>> compatible return type;
>> - typed class properties don't implicitly inherit the parent type when
>> the type left off a child property - they must declare the same type.
>> AFAIK there is no existing behaviour in PHP where omitting a type would
>> mean "the type is implicitly inherited from X", it either means the
>> same as mixed, or it's an error.
>=20
> That to me suggests that IF a custom variable name is provided, we should r=
equire also specifying the type.  In which case, in the 95% case, if we requ=
ire the full argument signature then the 95% case would need to double-speci=
fy the type, which is a hard-no from an ergonomic standpoint.
>=20
> Especially combined with the suggestion yesterday to allow return-to-set i=
n the short-set version, that would mean comparing this:
>=20
> public string $phone {
>   set(string $phone) =3D> $this->phone =3D $this->sanitizePhone($phone);
> }
>=20
> To this:
>=20
> public string $phone {
>   set =3D> $this->sanitizePhone($value);
> }
>=20
> And to me, there's absolutely no contest.  The latter has about 1/3 as man=
y places for me to make a typo repeating the same information over again.  N=
ow imagine comparing the above in a property that's used with constructor pr=
omotion.
>=20
>=20
> public function __construct(
>   public string $phone { set(string $phone) =3D> $this->phone =3D $this->s=
anitizePhone($phone); }
>   public string $phone { set =3D> $this->sanitizePhone($value); }
> ) {}
>=20
> Again, it's absolutely no contest for me.  I would detest writing the long=
er version every time.

I think you're making slightly misleading comparisons there, by picking the t=
wo extremes (and ignoring the possibly for shorter explicit names) - the exp=
licit parameter name surely doesn't preclude the use of return-to-set functi=
onality? So the comparison could equally be:

```
public function __construct(
   public string $phone { set =3D> $this->sanitizePhone($value); }
   public string $phone { set(string $v) =3D> $this->sanitizePhone($v); }
){}

```


> If PHP has been moving away from weird and inexplicable magic, it's also b=
een moving away from needless boilerplate.  (Constructor promotion being the=
 best example, but not the only; types themselves are a factor here, as are a=
rrow functions.)  As the whole point of this RFC is to make writing common c=
ode easier, requiring redundant boilerplate for it to work is actively count=
er-productive.
>=20
> So what I'd suggest instead is "specify the full signature if you want a c=
ustom name OR wider type; or omit the param list entirely to get a same-type=
 $value variable, which 99% of the time is all you need."  We get that in re=
turn for documenting "$value is the default", which for someone who has alre=
ady figured out $this, should be a very low effort to learn.

I get your point, and to expand on what I said in the first email - if remov=
ing the implicit mode would mean people vote against the RFC, then that's a w=
orse result, IMO (this is why I suggest a secondary vote - perfect is th ene=
my of good and all that). I personally think the implicit variables will res=
ult in less-readable code, but I also know that I'm free to not use the impl=
icit parameter in code that I write, so I trust those with a more accurate f=
inger on the pulse of the voters to know whether the implicit `$value` param=
eter will help or hinder the RFC to pass.


>=20
>> Also, a small nitpick: The link to your attributeutils repo in the
>> examples page, is broken, and it would be nice to see a few examples
>> showing the explicit version of the hooks.
>=20
> Link fixed, thanks.  What do you mean explicit version of the hooks?

I meant hooks that don't use the implicit "magic" variables (i.e. specify th=
e parameter on set() specify the full property name when accessing the backi=
ng store). Given your first response I guess this request is a little moot, a=
ssuming the examples are updated to remove use of `$field`. Maybe the RFC or=
 an example already covers it and I skipped over it, but it'd be good to mak=
e it clear *exactly* what the equivalent of an implicit `$value` parameter i=
s.

>=20
> --Larry Garfield


Cheers

Stephen=