Hi Rowan, all,

Thanks for your responses and comments.


> I think it's perfectly reasonable for CurlUrl to be designed the same way,
> regardless of what else we do with the extension.
>

As suggested by most of you, I created a new version of the proposed new
URL API with a friendlier interface [1]. A quick descriptions of the
changes I made :
- The curl_url_* procedural API was removed
- The CurlUrl::set() and ::get() methods with the CURLUPART_ constants were
replaced by more explicit setters and getters (ex: setHost() and getHost()).
- All the CURLU_* constants are now CurlUrl class constants (ex:
CURLU_GUESS_SCHEME => CurlUrl::GUESS_SCHEME)
- All the CURLUE_* constants are now CurlUrlException class constants (ex:
CURLUE_MALFORMED_INPUT => CurlUrlException::MALFORMED_INPUT)

Does it look more like what you were expecting ? Any other improvements
that should be made ?

This feels like a very small gain, since users can already type "curl_" and
> get a list of all curl functions. It also has real costs: users may be
> confused why there are two ways of writing the same thing; and it sets the
> precedent that methods on CurlHandle should mirror curl_* functions, even
> though we know those have terrible usability.
>

This is not the only change that I would like to introduce with a new OO
API. One other change that I would like to introduce is a better type
checking of `CurlHandle::setOpt()` when declaring strict_types to 1 (I know
Sara once opened a PR about this [2]). The current signature of the
procedural API is `curl_setopt(CurlHandle $handle, int $option, mixed
$value): bool`. The 3rd argument being mixed never throws any TypeError,
and adding this to the current procedural API would be a breaking change.
If we add a new OOP API this kind of change is possible and we can improve
the error message like : "Argument 2 passed to CurlHandle::setOpt() for
option CURLOPT_BUFFERSIZE must be of type int, string given"

However, knowing that the feature freeze for 8.2 is in a month, I will put
more emphasis on the Curl URL API than on a new OO API for existing Curl
API.

Thanks again for your feedback

Pierrick

[1] https://github.com/adoy/php-src/pull/1/files
[2] https://github.com/php/php-src/pull/2495

Reply via email to