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 <[email protected]> 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