Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122375
Content-Type: multipart/alternative;
 boundary="------------IkxUJFpfbJgO3r58eFk0MjeB"
Message-ID: <94983c1c-fa40-4a6e-8c5a-d3e10ecea856@wikimedia.org>
Date: Thu, 15 Feb 2024 14:34:36 +1100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC] OOP API for cURL extension
Content-Language: en-US
To: Sara Golemon <pollita@php.net>, PHP internals <internals@lists.php.net>
References: <CAESVnVp11-EVBrhzSj16=Nd3fYE366piq6fG-9wV3+iH8TxchA@mail.gmail.com>
In-Reply-To: <CAESVnVp11-EVBrhzSj16=Nd3fYE366piq6fG-9wV3+iH8TxchA@mail.gmail.com>
From: tstarling@wikimedia.org (Tim Starling)

This is a multi-part message in MIME format.
--------------IkxUJFpfbJgO3r58eFk0MjeB
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 15/2/24 05:47, Sara Golemon wrote:
> Good afternoon folks, I'd like to open discussion on adding OOP APIs 
> to the cURL extension.
> https://wiki.php.net/rfc/curl-oop
>
Thanks for making this. It's certainly an improvement, but it's 
disappointing to see such a conservative change when the extension 
interface is so weird and awkward. I'm sure you know what I mean.

It's easy to forget how bad it is until you have to do something 
without a framework, or explain it to a new developer.

I guess the proper fix would be to add a new extension with new names 
for everything, with Request and Response objects, along the lines of 
PSR-18. But if I can think of a few minor backwards-compatible changes 
which would improve things a bit. For example, getInfo() could be 
split out to separate read-only properties. And setOpt() could be 
split out to separate chainable mutator methods. That would improve 
type inference in surrounding code.

I see CurlHandle::exec() does not have the same return value as 
curl_exec() in your proposal, since errors are promoted to exceptions. 
But it still has a union return type. I would take it one step 
further, and have it unconditionally return either a string or void.

Having it return a string is defensible if you consider it to be 
returning the final value of the body buffer. Enable 
CURLOPT_RETURNTRANSFER by default. If CURLOPT_WRITEFUNCTION or 
CURLOPT_FILE is set and thus the buffer doesn't get appended to, 
naturally curl_exec() will return an empty string.

-- Tim Starling

--------------IkxUJFpfbJgO3r58eFk0MjeB
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 7bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 15/2/24 05:47, Sara Golemon wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAESVnVp11-EVBrhzSj16=Nd3fYE366piq6fG-9wV3+iH8TxchA@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">Good afternoon folks, I'd like to open discussion
        on adding OOP APIs to the cURL extension.<br>
        <a href="https://wiki.php.net/rfc/curl-oop"
          moz-do-not-send="true" class="moz-txt-link-freetext">https://wiki.php.net/rfc/curl-oop</a><br>
        <br>
      </div>
    </blockquote>
    <p>Thanks for making this. It's certainly an improvement, but it's
      disappointing to see such a conservative change when the extension
      interface is so weird and awkward. I'm sure you know what I mean.<br>
    </p>
    <p>It's easy to forget how bad it is until you have to do something
      without a framework, or explain it to a new developer.</p>
    <p>I guess the proper fix would be to add a new extension with new
      names for everything, with Request and Response objects, along the
      lines of PSR-18. But if I can think of a few minor
      backwards-compatible changes which would improve things a bit. For
      example, getInfo() could be split out to separate read-only
      properties. And setOpt() could be split out to separate chainable
      mutator methods. That would improve type inference in surrounding
      code.<br>
    </p>
    <p>I see CurlHandle::exec() does not have the same return value as
      curl_exec() in your proposal, since errors are promoted to
      exceptions. But it still has a union return type. I would take it
      one step further, and have it unconditionally return either a
      string or void.</p>
    <p>Having it return a string is defensible if you consider it to be
      returning the final value of the body buffer. Enable
      CURLOPT_RETURNTRANSFER by default. If CURLOPT_WRITEFUNCTION or
      CURLOPT_FILE is set and thus the buffer doesn't get appended to,
      naturally curl_exec() will return an empty string.<br>
    </p>
    <p>-- Tim Starling<br>
    </p>
  </body>
</html>

--------------IkxUJFpfbJgO3r58eFk0MjeB--