Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120428
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-441-ga3ab13cd6d-fm-20230517.001-ga3ab13cd
Mime-Version: 1.0
Message-ID: <280bcea8-9483-4191-80d3-81763a988290@app.fastmail.com>
In-Reply-To: 
 <CABdc3WqsVq4Jo1tt5EOevQ15aXhLA1+6SRmSENc8-K7SHX20Hg@mail.gmail.com>
References: 
 <CAH5C8xUn2wotLg0L7KZHoOGf3q-UqbL=47p2PLzK_mOBb=MJXQ@mail.gmail.com>
 <CAOWwgpncdhKvhwiAFUh_c_8+_vr=u9HhYxws_WZArh7W3Fi0xw@mail.gmail.com>
 <799ae864-6e25-4196-a5ce-0d74600a8378@app.fastmail.com>
 <CAAwdEzCbRdDHN=WYZy_jnyrM9USG9k9bxn=AW-KVWB8iSrBmrQ@mail.gmail.com>
 <CAOWwgp=Ch2iwOzxooiRcfk3WYMz-eYmQ51M-UP2Hh3bDogX5uw@mail.gmail.com>
 <CAH5C8xV76ou5Edpvq-vzb6RytW5-3uGanX+Sqf92-qnH26ccBw@mail.gmail.com>
 <CABdc3WqsVq4Jo1tt5EOevQ15aXhLA1+6SRmSENc8-K7SHX20Hg@mail.gmail.com>
Date: Mon, 29 May 2023 09:35:56 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [RFC] [Discussion] Clone with
From: larry@garfieldtech.com ("Larry Garfield")

On Mon, May 29, 2023, at 6:47 AM, Micha=C5=82 Marcin Brzuchalski wrote:
> Hi M=C3=A1t=C3=A9,
>
> pon., 29 maj 2023 o 11:18 M=C3=A1t=C3=A9 Kocsis <kocsismate90@gmail.co=
m> napisa=C5=82(a):
>
>> Hi Everyone,
>>
>> In the meanwhile, I changed my proposal to use [] instead of {} after=
 the
>> "with" clause due to its better receptance.
>> Additionally, I removed support for the shorthand "property assignmen=
t"
>> syntax (clone $this with [property1: "foo"]) in
>> favor the more powerful one where the left-hand side supports express=
ions
>> (clone $this with ["property1" =3D> "foo"])
>> so that we have more time to decide whether the former one is really
>> needed.
>>
>
> So there would be no option to vote on shorthand properties, right?
>
> Array syntax with all properties as strings in quotes probably means no
> support from IDE.
> I think this is a really bad move, I'd be against it.

I concur, especially with a dynamic list pushed off to future scope.  As=
 is, we're getting less IDE friendliness and a lot more ' characters in =
return for making the property name dynamic.  However, the *number* of p=
roperty names is still fixed at code time.  That's an incomplete tradeof=
f.

Conversely, using the named arguments style syntax (what future scope ca=
lls a shorthand, which I don't think is accurate as it's nearly the same=
 amount of typing) and allowing it to be "splatted", just like named arg=
uments, gives us the best of all worlds.  The typical case is easy to re=
ad, easy to write, and IDE-refactor-friendly.  Allowing it to be splatte=
d (just like named arguments) means that in the unusual case where you w=
ant the number or names of the properties to be dynamic (which is a vali=
d use case, but a minority one), you can pre-build an array and splat it=
 in place, exactly like you can named arguments.

Typical:

return clone $this with (
  statusCode: $code,
);

Fancy:

$props[strtolower('statusCode')] =3D $code;
if ($reason) {
  $props['reason'] =3D $reason;
}
return clone $this with ...$props;

That approach, taken now, gives us all the flexibility people have been =
asking for, reuses the existing named arguments syntax entirely, and is =
the most refactorable option.

Additionally, the current "property names expression" section shows a ve=
ry odd example.  It's recloning the object N times, reinvoking the __clo=
ne method each time, and reassigning a single property.  If you're updat=
ing several properties at once, that is extremely inefficient.  Dynamic =
property names are not the solution there; allowing the list to be dynam=
ic in the first place is, and that should not be punted to future scope.

I really want clone-with, but this seems like a big step backwards in te=
rms of its capability.  It may be enough for me to vote no on it, but I'=
m not sure.

(Pushing clone-with-callable off to later is, I agree, the correct move.)

I am also confused by the choice to make __clone() run before with with =
properties.  I would have expected it to be the other way around.  Can y=
ou explain (in the RFC) why you did it that way?  That seems more limiti=
ng than the alternative, as you cannot forward-pass information to __clo=
ne() this way.

--Larry Garfield