Thanks for the in-depth response, Matthew.

I missed the fact that only detach() is actually part of the spec, but even
with your clarifications, I still have issues with the detach() method.

> The reason for having detach() was to allow access to the underlying
> resource in order to perform operations not supported by the
> interface.

This is my main problem right here - the way I hear that explanation is, to
put it crudely, "when the abstraction fails, you can drop down and rely on
an implementation detail", which sounds to me like a plain description of
an abstraction that doesn't work.

You provide two concrete examples:

> As an example, if you want to use stream_copy_to_stream()
> to copy the stream to PHP's php://output stream more performantly, or
> to copy an incoming stream to an HTTP client request stream, you would
> need access to the underlying resource.

In my opinion, such as optimization should be an implementation detail in
the StreamInterface implementations.

If the requirement is to be able to copy one stream to another, the
interface should have had a copyTo() method - an implementation backed by
stream-resource handles could then make the optimization internally, and
fall back to manually copying until the stream ends.

> Similarly, if you wanted to
> use iterator_to_array() to cast an iterator- or generator-backed
> stream to an array, you could pull the underlying resource.

Knowing that an implementation is backed by an iterator or generator again
relies on knowledge of an implementation detail - it basically requires
knowledge of precisely which implementation you're using, which, again,
means the abstraction has failed.

The whole point of an abstraction is you don't need to know which
implementation you're using.

But really, my main issue is with the fact that detach() leaves the stream
in an invalid state - we have an abstraction here whose sole responsibility
is to represent a stream, and by allowing the internal stream-handle to be
removed, it ends up effectively no longer representing a stream, the only
responsibility it had.

In my opinion, the aim for this abstraction was set too low. It seems the
goal was to provide a utility wrapper around resource stream-handles - I
know from ready the meta-doc that was in fact the objective.

So what we have is only a partial abstraction, really - and for "things not
covered by that abstraction" we let you drop down to the underlying
stream-handle, an implementation detail.

In my opinion, the goal should have been an interface that fully abstracts
stream-handles - you shouldn't need to know how it's implemented, a
resource-handle should never be exposed, and the reference to the
resource-handle should be removed only automatically when the object is
destroyed.

The fact that things like fgetcsv() doesn't work for an abstraction like
that is the price you pay for abstracting streams in the first place -
you'll have to use str_getcsv() instead, but in my opinion, that's much
better and cleaner than relying on implementation details.

Look at this code, for example:

https://github.com/middlewares/payload/blob/master/src/CsvPayload.php

It's completely dependent on implementation details and side-effects:

1. it won't work at all unless the stream implementation has a
resource-handle,
2. it won't work if called twice with the same resource-handle, and
3. will cause other code to fail (depending on execution order)

In my opinion, that's the sort of mess this feature encourages, and it
shouldn't exist.

If there is ever a stand-alone stream PSR that supersedes this, I hope it's
designed as a full rather than partial replacement for streams...


On Sat, Dec 31, 2016 at 7:26 PM, Matthew Weier O'Phinney <
[email protected]> wrote:

> On Thu, Dec 29, 2016 at 7:16 AM, Rasmus Schultz <[email protected]>
> wrote:
> > Reading through those comments, I am if anything now more baffled.
> >
> > Looking at an implementation of attach() such as zend-diactoros, the
> > implementation of attach() is precisely identical to that of
> __construct() -
> > it effectively permits you to replace the entire object with a different
> > object, e.g. provides a kind of mutability that enables you to replace
> the
> > entire object.
> >
> > As far as I can figure, the only thing you can hope to accomplish with
> that,
> > is side-effects - a degree of side-effects not even possible with native
> > streams.
> >
> > And we wanted to avoid side-effects, right? That's why the rest of the
> model
> > is immutable.
> >
> > Can you point me to a real-world example of attach() and detach() in use?
>
> attach() exists in zend-diactoros because the library was developed in
> tandem with PSR-7 development. Unfortunately, the method was never
> removed. I plan to do so in the next major revision, and will be
> marking it as deprecated in an upcoming minor release. Please ignore
> the method when considering PSR-7; we omitted it from the spec for a
> number of reasons.
>
> First, as you note, it goes against the design of the rest of the
> specification, which strives for immutability. Second, replacing the
> underlying resource is something that is untenable: not all stream
> implementations will be backed with the same type of resource, making
> implementation essentially impossible. As an example, while the target
> implementation is a PHP stream resource, we have also allowed for
> callback, generator, and iterator-backed stream. What happens if you
> provide one of these to an implementation that only supports PHP
> streams? or a PHP stream to an implementation that expects a callback?
>
> As such, we removed attach() from the spec. As I've also noted, I
> neglected to remove it from Diactoros when that change happened. It's
> not part of the spec; as a PSR-7 consumer, it's something you can
> safely ignore.
>
> The reason for having detach() was to allow access to the underlying
> resource in order to perform operations not supported by the
> interface. As an example, if you want to use stream_copy_to_stream()
> to copy the stream to PHP's php://output stream more performantly, or
> to copy an incoming stream to an HTTP client request stream, you would
> need access to the underlying resource. Similarly, if you wanted to
> use iterator_to_array() to cast an iterator- or generator-backed
> stream to an array, you could pull the underlying resource. detach()
> is _destructive_, however, because any such manipulations could
> themselves alter the behavior of the underlying resource associated
> with the StreamInterface. As an example, a read-only stream, when
> read, would leave the StreamInterface implementation in unusable
> state.
>
> Hopefully the above helps clarify the purpose of detach(), and why
> attach() was removed from the specification.
>
> >
> >
> > On Thursday, December 29, 2016 at 1:48:42 PM UTC+1, Stefano Torresi
> wrote:
> >>
> >> Hey Rasmus,
> >>
> >> The detach() method was introduced to allow a fine grained control over
> >> the underlying resource of the Stream object, and it provides a
> standardized
> >> way to do that.
> >> You can find more insights in the original discussion here:
> >> https://groups.google.com/d/msg/php-fig/CTPRa2XP8po/kYZnFxl4JjMJ
> >>
> >> Admittedly, the metadoc is a bit lacking in this regard.
> >>
> >> I think we could amend it with a clarifying errata, couldn't we?
> >>
> >> Il giorno gio 29 dic 2016 alle ore 13:25 Rasmus Schultz
> >> <[email protected]> ha scritto:
> >>>
> >>> The detach() method of StreamInterface in PSR-7 isn't mentioned
> anywhere
> >>> in the documentation or meta.
> >>>
> >>> The only documentation for it, is the inline documentation, which
> doesn't
> >>> explain what it's for:
> >>>
> >>> > Separates any underlying resources from the stream.
> >>>
> >>> So basically, this sets the internal resource handle (an implementation
> >>> detail!) to null, right?
> >>>
> >>> > After the stream has been detached, the stream is in an unusable
> state.
> >>>
> >>> So if I want my stream object in an invalid, unusable state, this is
> the
> >>> method to call.
> >>>
> >>> Huh?
> >>>
> >>> When would I want that?
> >>>
> >>> Since streams are (primarily) intended as wrappers around PHP
> >>> stream-resource handles, I would have expected that the way to
> "detach" a
> >>> StreamInterface instance from the native stream-resource handle, is to
> >>> destroy the object - since a StreamInterface-wrapper around a PHP
> >>> stream-resource handle really represents only one thing: the
> >>> resource-handle.
> >>>
> >>> When would "detaching" and leaving the object in an invalid state make
> >>> more sense than simply destroying the object, and thereby the
> reference to
> >>> the stream-resource handle?
> >>>
> >>> --
> >>> You received this message because you are subscribed to the Google
> Groups
> >>> "PHP Framework Interoperability Group" group.
> >>> To unsubscribe from this group and stop receiving emails from it, send
> an
> >>> email to [email protected].
> >>> To post to this group, send email to [email protected].
> >>> To view this discussion on the web visit
> >>> https://groups.google.com/d/msgid/php-fig/99fd9037-331d-
> 4645-ad0f-c911ae370356%40googlegroups.com.
> >>> For more options, visit https://groups.google.com/d/optout.
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "PHP Framework Interoperability Group" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to [email protected].
> > To post to this group, send email to [email protected].
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/php-fig/08a6b4ab-fcec-
> 4ccc-8b0f-351cdf3612cd%40googlegroups.com.
> >
> > For more options, visit https://groups.google.com/d/optout.
>
>
>
> --
> Matthew Weier O'Phinney
> [email protected]
> https://mwop.net/
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this topic, visit https://groups.google.com/d/
> topic/php-fig/eoasCZSRmbI/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/php-fig/CAJp_myU8HNs8y4YbgOt%3DAUHH47tCbewsfZmQn%
> 2BnR0kO62W7hcQ%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "PHP 
Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/php-fig/CADqTB_j_N4p3-ojqhN0bri2hpgZLFFmXjA5Lp16yPdA0j6pB8Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to