On Mon, May 29, 2023, at 6:47 AM, Michał Marcin Brzuchalski wrote:
> Hi Máté,
>
> pon., 29 maj 2023 o 11:18 Máté Kocsis <kocsismat...@gmail.com> napisał(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 assignment"
>> syntax (clone $this with [property1: "foo"]) in
>> favor the more powerful one where the left-hand side supports expressions
>> (clone $this with ["property1" => "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 property names is 
still fixed at code time.  That's an incomplete tradeoff.

Conversely, using the named arguments style syntax (what future scope calls 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 arguments, gives us 
the best of all worlds.  The typical case is easy to read, easy to write, and 
IDE-refactor-friendly.  Allowing it to be splatted (just like named arguments) 
means that in the unusual case where you want the number or names of the 
properties to be dynamic (which is a valid 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')] = $code;
if ($reason) {
  $props['reason'] = $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 very odd 
example.  It's recloning the object N times, reinvoking the __clone method each 
time, and reassigning a single property.  If you're updating several properties 
at once, that is extremely inefficient.  Dynamic property names are not the 
solution there; allowing the list to be dynamic 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 terms 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 you 
explain (in the RFC) why you did it that way?  That seems more limiting than 
the alternative, as you cannot forward-pass information to __clone() this way.

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to