Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122386
Precedence: bulk
MIME-Version: 1.0
References: <CAESVnVp11-EVBrhzSj16=Nd3fYE366piq6fG-9wV3+iH8TxchA@mail.gmail.com>
In-Reply-To: <CAESVnVp11-EVBrhzSj16=Nd3fYE366piq6fG-9wV3+iH8TxchA@mail.gmail.com>
Date: Thu, 15 Feb 2024 08:44:13 -0700
Message-ID: <CAESVnVpHRU8-Fc10XEdRkf3tU-5bXxO4tZ-CKwWJeUabB2LM4g@mail.gmail.com>
Subject: [PHP-DEV] Re: [RFC] OOP API for cURL extension
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000007423c906116d8094"
From: pollita@php.net (Sara Golemon)

--0000000000007423c906116d8094
Content-Type: text/plain; charset="UTF-8"

Summarizing replies so far.  Won't be able to update the RFC immediately as
my day job needs me, but some great discussions already, gang. Thanks!

* Define the conditions under which exceptions will be thrown (and which
exceptions) - I'll add these to the RFC, but in short:
  * CurlException - Never, it's an interface type to group the other
exceptions.
  * CurlHandleException - Whenever a CurlHandle::method() fails (in lieu of
returning false)
  * CurlMultiException - Same, but for the CurlMultiHandle class.
  * CurlShareException - Same, but for the CurlShareHandle class.

* Naming styles, e.g getErrorNumber(): int   vs   errno()
  Comment: Agreed with your points.  We don't have to stick to a hard line
mapping of the function names.  My initial translation did because my
bugbear is with the lack of fluency and all the rest is just window
dressing on that.
  Proposal: I'll rename all the new APIs according to a get*/set* scheme
without abbreviations, e.g.:
  * CurlHandle::getErrorNumber(): int
  * CurlHandle::getError(): ?string
  * static CurlHandle::getErrorTextFromCode(int $code): ?string
  * CurlHandle::execute(): ?string  // See late note about exec return
values
  * CurlHandle::setOptionInt(int $option, int $value): CurlHandle

* Better typing for setOpt() methods.
  Comment: Yep, this is a good and fair point.  It's going to take a little
more dicing up of the current implementation, but hopefully not too rough.
  Proposal: Keep untyped setOption() method (1/ Easier migration of
existing code, 2/ Some options may prove difficult to type), but add:
  * CurlHandle::setOptionBool(int $option, bool $value): CurlHandle
  * CurlHandle::setOptionInt(int $option, int $value): CurlHandle
  * CurlHandle::setOptionString(int $option, string $value): CurlHandle
  * etc... as needed, will this get weird when we get to Array since there
IS a setOptArray?  Maybe we just don't mirror that one, or we call it
setOptionMany(). Yeah, I like Many for the multiple option set, and Array
for the single option of array type.
  Internally, the setopt handler will be split by type so that each typed
setting can call explicitly, and the untyped one can call all.

* CurlHandle::exec() mixed typing of return values.
  Comment: Agreed.  The `true` return value becomes meaningless in the
RETURNTRANSFER==false case.
  Proposal: Update the RFC for CurlHandle::execute() to return ?string.

* https://php.watch/articles/php-curl-security-hardening
  Comment: I'll read this later when I'm updating the RFC.

* Prefer class constants to global constants.
  Comment: I'm less compelled by this.  The global scope is polluted with
the constants whether we add copies or not.  Adding a second way to spell
the constant name only adds a second way to spell the constant name.
  Proposal: Change my mind?

* Request and Response objects, along the lines of PSR-18
  Comment: I'd be in favor of that, but it's not the mountain I'm
personally trying to climb today.  No offense taken if you vote no because
this isn't enough, but I don't have the cycles to go in that hard.
   Proposal: Write an RFC (and get it passed) and I can probably help you
implement it?

-Sara

--0000000000007423c906116d8094
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Summarizing replies so far.=C2=A0 Won&#39;t be able t=
o update the RFC immediately as my day job needs me, but some great discuss=
ions already, gang. Thanks!</div><div><br></div><div>* Define the condition=
s under which exceptions will be thrown (and which exceptions) - I&#39;ll a=
dd these to the RFC, but in short:</div><div>=C2=A0 * CurlException - Never=
, it&#39;s an interface type to group the other exceptions.</div><div>=C2=
=A0 * CurlHandleException - Whenever a CurlHandle::method() fails (in lieu =
of returning false)<br>=C2=A0 * CurlMultiException - Same, but for the Curl=
MultiHandle class.<br>=C2=A0 * CurlShareException - Same, but for the CurlS=
hareHandle class.</div><div><br>* Naming styles, e.g getErrorNumber(): int=
=C2=A0 =C2=A0vs=C2=A0 =C2=A0errno()<br>=C2=A0 Comment: Agreed with your poi=
nts.=C2=A0 We don&#39;t have to stick to a hard line mapping of the functio=
n names.=C2=A0 My initial translation did because my bugbear is with the la=
ck of fluency and all the rest is just window dressing on that.<br>=C2=A0 P=
roposal: I&#39;ll rename all the new APIs according to a get*/set* scheme w=
ithout abbreviations, e.g.:</div><div>=C2=A0 * CurlHandle::getErrorNumber()=
: int</div><div>=C2=A0 * CurlHandle::getError(): ?string</div><div>=C2=A0 *=
 static CurlHandle::getErrorTextFromCode(int $code): ?string<br>=C2=A0 * Cu=
rlHandle::execute(): ?string=C2=A0 // See late note about exec return value=
s<br>=C2=A0 * CurlHandle::setOptionInt(int $option, int $value): CurlHandle=
</div><div><br></div><div>* Better typing for setOpt() methods.</div><div>=
=C2=A0 Comment: Yep, this is a good and fair point.=C2=A0 It&#39;s going to=
 take a little more dicing up of the current implementation, but hopefully =
not too rough.</div><div>=C2=A0 Proposal: Keep untyped setOption() method (=
1/ Easier migration of existing code, 2/ Some options may prove difficult t=
o type), but add:</div><div>=C2=A0 * CurlHandle::setOptionBool(int $option,=
 bool $value): CurlHandle</div><div>=C2=A0 * CurlHandle::setOptionInt(int $=
option, int $value): CurlHandle</div><div>=C2=A0 * CurlHandle::setOptionStr=
ing(int $option, string $value): CurlHandle</div><div>=C2=A0 * etc... as ne=
eded, will this get weird when we get to Array since there IS a setOptArray=
?=C2=A0 Maybe we just don&#39;t mirror that one, or we call it setOptionMan=
y(). Yeah, I like Many for the multiple option set,=C2=A0and Array for the =
single option of array type.</div><div>=C2=A0 Internally, the setopt handle=
r will be split by type so that each typed setting can call explicitly, and=
 the untyped one can call all.<br><br>* CurlHandle::exec() mixed typing of =
return values.</div><div>=C2=A0 Comment: Agreed.=C2=A0 The `true` return va=
lue becomes meaningless in the RETURNTRANSFER=3D=3Dfalse case.</div><div>=
=C2=A0 Proposal: Update the RFC for CurlHandle::execute() to return ?string=
.</div><div><br>*=C2=A0<a href=3D"https://php.watch/articles/php-curl-secur=
ity-hardening" rel=3D"noreferrer" target=3D"_blank">https://php.watch/artic=
les/php-curl-security-hardening</a><br>=C2=A0 Comment: I&#39;ll read this l=
ater when I&#39;m updating the RFC.<br><br></div><div>* Prefer class consta=
nts to global constants.</div><div>=C2=A0 Comment: I&#39;m less compelled b=
y this.=C2=A0 The global scope is polluted with the constants whether we ad=
d copies or not.=C2=A0 Adding a second way to spell the constant name only =
adds a second way to spell the constant name.</div><div>=C2=A0 Proposal: Ch=
ange my mind?</div><div><br></div><div>* Request and Response objects, alon=
g the lines of PSR-18</div><div>=C2=A0 Comment: I&#39;d be in favor of that=
, but it&#39;s not the mountain I&#39;m personally trying to climb today.=
=C2=A0 No offense taken if you vote no because this isn&#39;t enough, but I=
 don&#39;t have the cycles to go in that hard.</div><div>=C2=A0 =C2=A0Propo=
sal: Write an RFC (and get it passed) and I can probably help you implement=
 it?</div><div><br></div><div>-Sara</div><div><br></div><div class=3D"gmail=
_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex=
;border-left:1px solid rgb(204,204,204);padding-left:1ex">
</blockquote></div></div>

--0000000000007423c906116d8094--