On Tue, May 21, 2024 at 12:58 PM Claude Pache <claude.pa...@gmail.com>
wrote:

>
>
> > Le 18 mai 2024 à 01:13, Niels Dossche <dossche.ni...@gmail.com> a écrit
> :
> >
> > On 22/04/2024 20:41, Niels Dossche wrote:
> >> Hi internals
> >>
> >> I'm opening the discussion for my RFC "Add openStream() to
> XML{Reader,Writer}".
> >> RFC link: https://wiki.php.net/rfc/xmlreader_writer_streams
> >>
> >> Kind regards
> >> Niels
> >
> > Hi internals
> >
> > The main complaint that kept coming up in internal communication was the
> choice of instance methods instead of static methods.
> > This has been brought up in this thread too.
>
> Hi,
>
> Now you have a complaint because of the choice of static methods instead
> of instance methods... :-)
>
> You are introducing an inconstancy for subclasses, because:
>
> * As noted in an earlier message, the existing open-like methods
> (XMLReader::XML() and XMLReader::open()) don’t work statically on
> subclasses (they create an instance of the base class, not of the
> subclass). You must use them as instance methods.
>
> * On the other hand, the method you are going to introduce won’t work as
> instance method. You must use it statically.
>
> Please, if you want to make the new method static only, fix first the
> existing ones, so that they also work statically (on subclasses).
>
> (But again, I prefer that all those methods work on instances, as it was
> the case before PHP 8. They shouldn’t have been switched to
> static-but-broken-for-subclasses without discussion.)
>

Fixing the existing ones would be a potential BC break, depending on
whether or not instance usage is completely eliminated.

The reason why many of us recommended static methods here is that they
would act as named constructors, and reduce the potential for cases where a
new stream is read from or written to mid-flight. Essentially, the usage
becomes:

    $reader = XMLReader::fromStream($streamHandle);
     // read the stream

and

    $writer = XMLWriter::toStream($streamHandle);
    // write to the stream

My understanding is that the implementations will allow for subclassing.

Having these as instance methods means that subclasses would need to
potentially add handling to ensure the instance is not in an invalid state
(e.g., by switching streams mid-flight), which would add far more edge
cases to handle.

-- 
Matthew Weier O'Phinney
mweierophin...@gmail.com
https://mwop.net/
he/him

Reply via email to