Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110009
MIME-Version: 1.0
References: <CAF+90c8m-CB3ZxAHeoOJVxJGhysLfZqpz3mq475GPnMEDA6iqQ@mail.gmail.com>
 <CADyq6s+pfDfDey8hGT=zDD5hwyKjq3q6V8xPVyP03z=6MwGgdQ@mail.gmail.com> <CAEPJdunAY1taDC3oR+3faUj5wfFxAGVTC41G8=WkA-qfG1HrDw@mail.gmail.com>
In-Reply-To: <CAEPJdunAY1taDC3oR+3faUj5wfFxAGVTC41G8=WkA-qfG1HrDw@mail.gmail.com>
Date: Tue, 5 May 2020 17:55:40 +0200
Message-ID: <CADyq6sLJHjML=TvZKo3k9WfEcnAS4NZc+dSZ_Adx-0CU2Uc28g@mail.gmail.com>
To: Benjamin Eberlei <kontakt@beberlei.de>
Cc: Nikita Popov <nikita.ppv@gmail.com>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a3db7d05a4e8aed8"
Subject: Re: [PHP-DEV] [RFC] Named arguments
From: ocramius@gmail.com (Marco Pivetta)

--000000000000a3db7d05a4e8aed8
Content-Type: text/plain; charset="UTF-8"

Hey Benjamin,

On Tue, May 5, 2020 at 5:44 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

> I see the PHP internal API as the main benefactor of this feature over
> userland code imho. Internals realistically couldn't get "smaller APIs"
> without adding many new functions instead, for example htmlspecialchars.
>

It takes a couple minutes to write a non-broken `htmlspecialchars` function
to use in your own project.

Besides that, designing a language feature to accommodate pre-existing bad
design smells a lot.

There are libraries out there trying to provide better API to what is
provided by PHP-SRC, such as https://github.com/thecodingmachine/safe:
that's an easy win, by creating **adapters** for bad API, instead of
**language-level constructs** that will haunt us forever.


> The trade off from API design perspective is either providing a more
> complex API by having function permutations or an OOP API, which in turn
> developers have more trouble remembering. Or the more beginner approachable
> way is providing a single function that is easy to remember exists, but in
> turn does more things and may require a look into the documentation to see
> all the options. Given we have this API already in PHP and this will not
> realistically change, I think named params will be a big win.
>

Sorry, but no: you provide multiple constructors, each with a limited set
of arguments, each guaranteeing constraints that depend on context.

For instance, a `fromArray()` may check for key existence, while
`fromJson()` may use a JSON-Schema definition to check that all is as
required, and `fromXmlPayload()` may apply an XSD validator, and so on...

Number, type and lifecycle/mutability of parameters may change depending on
named constructor, and the reduced scope makes it very easy to document
requirements, and to make the API ergonomic (yes, it's actually easier to
use: surprise!).

In this example $stuff is an array which has keys. Presumably
> $parameterName => $newParameterName would be equivalent to a change of key
> names. Or property names on
> MyInfiniteSequenceOfParametersAsProperValidatedStructure. So you get the BC
> break of renaming anyways in your example.
>

Correct: that's a BC break on my
`MyInfiniteSequenceOfParametersAsProperValidatedStructure` which does not
depend on the definition of `myHorribleLegacyAndOrganicallyGrownApi()`.

It is still a very well isolated BC scenario on a single `::fromArray()`
named ctor, compared to introducing a language-level BC boundary that
crosses all existing code out there.

Now as a provider of a public API (library, framework) you can still
> counter this with a BC layer:
>
> function foo($newParameterName, $parameterName = null) {
>     if ($parameterName !== null) {
>        $newParameterName = $parameterName;
>        trigger_error("Using paramaterName as named parameter is
> deprecated, use newParameterName instead.", E_USER_DEPRECATED);
>     }
>     /...
> }
>
> this looks ugly (because we haven't seen it before), but is not more or
> less complex than the same BC / deprecation layer in fromArray with changed
> option:
>
> public static function fromArray(array $options) {
>    if (isset($options['parameter_name'])) {
>        $options['new_parameter_name'] = $options['parameter_name'];
>        trigger_error("Using paramaeter_name as option is deprecated, use
> new_parameter_name instead.", E_USER_DEPRECATED);
>    }
> }
>
> So nothing much would technically change for someone who cares about BC
> for their users.
>

In first place, I would rarely ever design a `::fromArray()` API that isn't
related to serialization or configuration loading (which is another kind of
serialization, after all).

I would not look forward to maintaining BC compliance like with
`foo($newParameterName, $parameterName = null)`, and I do not see which
added value counters this massive amount of maintenance overload.

We're chasing a non-existing problem (or feature requirement) by adding
other problems that are far greater.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

--000000000000a3db7d05a4e8aed8--