Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118087
MIME-Version: 1.0
References: <CAOfKkdTjPiZHnSHuag_+fmFK-1Advh2-vfDgbJBXEmK=72NB0g@mail.gmail.com>
 <CAAKU0um7-7KZnBibwQxkvm_Kq8a264BZYOPc8G2jfax_43odjQ@mail.gmail.com>
 <CAOfKkdTiptR5i6+9x4UnV0sZeZh2Q8u167onzEHVwMq1Cv-E5A@mail.gmail.com> <fe328c01f99b49e6b90a8066f1b54dfc@fsx.com>
In-Reply-To: <fe328c01f99b49e6b90a8066f1b54dfc@fsx.com>
Date: Fri, 24 Jun 2022 16:36:42 +0100
Message-ID: <CALKiJKq3hccPgvUtkZdjrwqjeq3cXXUK-M7-30rwd31t=4ggqA@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000000446505e2335761"
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] New Curl URL API
From: rowan.collins@gmail.com (Rowan Tommins)

--00000000000000446505e2335761
Content-Type: text/plain; charset="UTF-8"

On Fri, 24 Jun 2022 at 15:05, Jeffrey Dafoe <JDafoe@fsx.com> wrote:

> A thin wrapper would be the most flexible. Someone can always write a
> "friendlier" class using your thin wrapper, as you mentioned, but one
> cannot easily go the other direction.
>


I think this argument has some validity when talking about the entire API
of a library like curl. But we're not, we're talking about a tiny bounded
context.

The underlying implementation consists of 6 functions, 4 of which are
trivial:

* An argument-less "constructor" https://curl.se/libcurl/c/curl_url.html
* An equivalent to PHP "clone" https://curl.se/libcurl/c/curl_url_dup.html
* A cleanup method, which would be automatic in any PHP implementation
https://curl.se/libcurl/c/curl_url_cleanup.html
* A function for looking up error strings from codes
https://curl.se/libcurl/c/curl_url_strerror.html

That leaves the main getter and setter functions
https://curl.se/libcurl/c/curl_url_get.html and
https://curl.se/libcurl/c/curl_url_set.html which take the same arguments:

* A resource handle
* The URL part to handle
* The new value for set, or an output parameter for get
* A set of flags

If we really believe the goal is to expose the C API in PHP, the get
function would look like this:

/**
 * @return int Error code
 */
function curl_url_get(resource $url, int $what, string &$part, int $flags):
int

That would be ... awful, and I hope nobody's really suggesting that. So we
have to map each part to some concept in PHP, which is what the proposal
does:

* The resource handle becomes $this
* The $what argument becomes the method name
* The output parameter becomes the return value
* The error code becomes an exception

There's no "flexibility" which has been lost, and no "future changes" which
are being prevented.


The only other change is a few renamed constants, which I suggested on the
PR. I can see an argument for making them match the library exactly; but
it's the *values* that actually matter, so I don't see why we shouldn't
choose our own names if they're descriptive of the functionality.


Regards,
-- 
Rowan Tommins
[IMSoP]

--00000000000000446505e2335761--