Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118098
MIME-Version: 1.0
References: <CAOfKkdTjPiZHnSHuag_+fmFK-1Advh2-vfDgbJBXEmK=72NB0g@mail.gmail.com>
In-Reply-To: <CAOfKkdTjPiZHnSHuag_+fmFK-1Advh2-vfDgbJBXEmK=72NB0g@mail.gmail.com>
Date: Mon, 27 Jun 2022 08:29:29 +0200
Message-ID: <CABdc3WryiOY=3cMTdi+gVxbE+twi8ixJGtG9Gfa+4RfJQT5DhQ@mail.gmail.com>
To: Pierrick Charron <pierrick@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000007b4bc405e2680b30"
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] New Curl URL API
From: michal.brzuchalski@gmail.com (=?UTF-8?Q?Micha=C5=82_Marcin_Brzuchalski?=)

--0000000000007b4bc405e2680b30
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Pierrick

=C5=9Br., 22 cze 2022 o 06:38 Pierrick Charron <pierrick@php.net> napisa=C5=
=82(a):

> Hi,
>
> Following our discussions we had on the subject of the new Curl URL API,
> and other curl improvements. I decided to only focus on adding the new Cu=
rl
> URL API and put aside all other improvements. Here is the RFC that reflec=
ts
> our current conversations.
>
> https://wiki.php.net/rfc/curl-url-api


TBH I have a problem with the proposed Curl* classes.
IMO they present a mix of responsibilities that I don't like.

CurlUrl is for me is a mix of Url/Uri object properties well known from
other userland implementations like the PSR one
with Uri specific like the host, scheme, port, path, query, fragment, etc.
but on the other hand, we have flags and options which
purpose is to pass Curl specific things but in IMO wrong place instead (for
instance Guzzle use separate argument in all methods for options and flags.
Secondly, it has some default logic like `setScheme` allows to put a scheme
but requires the supported one, with an exception that you can ignore
support checking by a flag - IMO this logic belongs to its consumer logic
and is not part of URL class logic and the `setPort` like above.
Next thing is that it again has all the getters and setters and is not
immutable. Why can't it be a simple constructor with read-only properties,
no getters, no setters? Maybe I miss some discussion about why here.


The same goes for CurlException which looks like is an exception for
handling some encoding of Url and not exactly to the Url properties itself.

But with all that said, if the target audience is only a user of low lever
`curl_*` functions rather than users of higher abstraction libraries like
Guzzle or Httplug then, I guess I don't care that much.
I'm only afraid that such a mix of responsibilities can lead to bad habits
when it gets to the separation of concerns by developers who get used to it
and won' see the problem as I do.

Cheers,
Micha=C5=82 Marcin Brzuchalski

--0000000000007b4bc405e2680b30--