Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97529
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.53 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <D7828F3D-C550-4632-8F43-274278C1D1F9@gmail.com>
References: <0D388F9E-98FA-4C1F-95FA-759C4D4731C6@gmail.com>
 <2A.04.04761.F9A9D585@pb1.pair.com> <2017010414120739532-pmjones88@gmail.com> <D7828F3D-C550-4632-8F43-274278C1D1F9@gmail.com>
Date: Fri, 6 Jan 2017 17:47:58 +0100
Message-ID: <CADyq6sJkzGW2RLtZfmP66yh_sn064i1oJeuV2kb1cmY+KQnJ1Q@mail.gmail.com>
To: Paul Jones <pmjones88@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=f403045f1b4ecd433205456fc762
Subject: Re: [PHP-DEV] Re: [RFC] ServerRequest and ServerResponse objects
From: ocramius@gmail.com (Marco Pivetta)

--f403045f1b4ecd433205456fc762
Content-Type: text/plain; charset=UTF-8

Hi Paul,

On 6 Jan 2017 3:19 p.m., "Paul Jones" <pmjones88@gmail.com> wrote:

Hi all,

Today marks two weeks since this RFC was introduced. As I said at the
start, it's the holidays, so I expect to keep the discussion period open
for longer than two weeks, but I am surprised at how little discussion
there has been.

I find it hard to imagine that all criticisms have been raised, and that
the remainder of the RFC is otherwise unobjectionable. So, if there are
further questions, please let me know!


As mentioned before, the fact that the RFC proposes *yet another*  (XKCD
927) request/response API simply increases fragmentation within the
ecosystem.

This is not useful, especially now that a common interoperability interface
for request and response that works quite decently and is getting traction
(PSR-7) exists.

Bundling the interface is no biggie, so I suggest steering away and going
that way, or else you are just adding API that everyone would just need to
build wasteful adapters for.

There are several technical issues too:

 * A magic constructor that fetches data from global state: add a factory
for that, leave the constructor open for modifications or new instantiation
ways, or make it private.
 * Magic read-only properties that go against the language semantics: make
'em accessors, or do whatever you want, but don't invent new language
semantics just for your own extension.
 * A series of non-exhaustive properties that give some "helper" API to see
if it's XHR or not, violating basic SRP. Are you abstracting a request or
what you want to do with the request? You can define separate utility
functions for that. Yes, functions.
 * As per current API, I'd expect the ctor to throw an exception when used
outside the web SAPI context
 * You use the *with* prefix for setting values, while you know exactly
that *with* was used to differentiate from a mutable and a non-mutable API
in the existing request abstractions: this makes things confusing again
 * There's no interface for these classes, and they are not marked as
final, as far as I can see. This means that users will extend them (unwise
choice), and with that they will have a class with reflection bits in the
extension (extremely bad idea!). An interface and final are needed here.

Anyway, this is just a partial review of what I got to read with a few
minutes of time, yet loads of issues are evident.

I am obviously defensive and biased here, but if something new is being
designed for inclusion in php-src, and it has to reach millions of
developers, it better be *reeeeeealli* good. The issues above are just the
tip of the iceberg, and many developers obviously didn't use nor try this
extension so far.

IMO needs another few design iterations, and a lot more adoption, in order
to become interesting. Orrrrr you provide us with a factory for an
implementation of these concepts that already has adoption, traction and
extremely careful design behind it.

Greets,

Marco Pivetta

--f403045f1b4ecd433205456fc762--