Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123385
Precedence: bulk
MIME-Version: 1.0
References: <b8513546-294d-482e-bd1c-292b455e66a8@gmail.com>
 <ac54fd4f-24da-4649-ad2d-3431244d447b@gmail.com> <37BF3748-4A18-4046-8CCD-4198094FE059@gmail.com>
In-Reply-To: <37BF3748-4A18-4046-8CCD-4198094FE059@gmail.com>
Date: Tue, 21 May 2024 14:00:21 -0500
Message-ID: <CAJp_myWSagrqv3uH6CNxx0qskEsq0Ds-7Srf6DwSvn+4MT8B+w@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add openStream() to XML{Reader,Writer}
To: Claude Pache <claude.pache@gmail.com>
Cc: Niels Dossche <dossche.niels@gmail.com>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000acba920618fb6e4f"
From: mweierophinney@gmail.com ("Matthew Weier O'Phinney")

--000000000000acba920618fb6e4f
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, May 21, 2024 at 12:58=E2=80=AFPM Claude Pache <claude.pache@gmail.c=
om>
wrote:

>
>
> > Le 18 mai 2024 =C3=A0 01:13, Niels Dossche <dossche.niels@gmail.com> a =
=C3=A9crit
> :
> >
> > 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 th=
e
> 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=E2=80=99t 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=E2=80=99t =
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=E2=80=99t 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 =3D XMLReader::fromStream($streamHandle);
     // read the stream

and

    $writer =3D 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.

--=20
Matthew Weier O'Phinney
mweierophinney@gmail.com
https://mwop.net/
he/him

--000000000000acba920618fb6e4f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Tue, May 21, 2024 at 12:58=E2=80=
=AFPM Claude Pache &lt;<a href=3D"mailto:claude.pache@gmail.com">claude.pac=
he@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex"><br>
<br>
&gt; Le 18 mai 2024 =C3=A0 01:13, Niels Dossche &lt;<a href=3D"mailto:dossc=
he.niels@gmail.com" target=3D"_blank">dossche.niels@gmail.com</a>&gt; a =C3=
=A9crit :<br>
&gt; <br>
&gt; On 22/04/2024 20:41, Niels Dossche wrote:<br>
&gt;&gt; Hi internals<br>
&gt;&gt; <br>
&gt;&gt; I&#39;m opening the discussion for my RFC &quot;Add openStream() t=
o XML{Reader,Writer}&quot;.<br>
&gt;&gt; RFC link: <a href=3D"https://wiki.php.net/rfc/xmlreader_writer_str=
eams" rel=3D"noreferrer" target=3D"_blank">https://wiki.php.net/rfc/xmlread=
er_writer_streams</a><br>
&gt;&gt; <br>
&gt;&gt; Kind regards<br>
&gt;&gt; Niels<br>
&gt; <br>
&gt; Hi internals<br>
&gt; <br>
&gt; The main complaint that kept coming up in internal communication was t=
he choice of instance methods instead of static methods.<br>
&gt; This has been brought up in this thread too.<br>
<br>
Hi,<br>
<br>
Now you have a complaint because of the choice of static methods instead of=
 instance methods... :-)<br>
<br>
You are introducing an inconstancy for subclasses, because:<br>
<br>
* As noted in an earlier message, the existing open-like methods (XMLReader=
::XML() and XMLReader::open()) don=E2=80=99t work statically on subclasses =
(they create an instance of the base class, not of the subclass). You must =
use them as instance methods.<br>
<br>
* On the other hand, the method you are going to introduce won=E2=80=99t wo=
rk as instance method. You must use it statically.<br>
<br>
Please, if you want to make the new method static only, fix first the exist=
ing ones, so that they also work statically (on subclasses).<br>
<br>
(But again, I prefer that all those methods work on instances, as it was th=
e case before PHP 8. They shouldn=E2=80=99t have been switched to static-bu=
t-broken-for-subclasses without discussion.)<br></blockquote><div><br></div=
><div>Fixing the existing ones would be a potential BC break, depending on =
whether or not instance usage is completely eliminated.</div><div><br></div=
><div>The reason why many of us recommended static methods here is that the=
y 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:</div><div><br></div><div>=C2=A0 =C2=A0 $reader =3D XMLReader::fro=
mStream($streamHandle);</div><div>=C2=A0 =C2=A0 =C2=A0// read the stream</d=
iv><div><br></div><div>and</div><div><br></div><div>=C2=A0 =C2=A0 $writer =
=3D XMLWriter::toStream($streamHandle);</div><div>=C2=A0 =C2=A0 // write to=
 the stream</div><div><br></div><div>My understanding is that the implement=
ations will allow for subclassing.</div><div><br></div><div>Having these as=
 instance methods means that subclasses would need to potentially add handl=
ing to ensure the instance is not in an invalid state (e.g., by switching s=
treams mid-flight), which would add far more edge cases to handle.</div></d=
iv><div><br></div><span class=3D"gmail_signature_prefix">-- </span><br><div=
 dir=3D"ltr" class=3D"gmail_signature"><div dir=3D"ltr"><div>Matthew Weier =
O&#39;Phinney<br><a href=3D"mailto:mweierophinney@gmail.com" target=3D"_bla=
nk">mweierophinney@gmail.com</a><br><a href=3D"https://mwop.net/" target=3D=
"_blank">https://mwop.net/</a></div><div>he/him<br></div></div></div></div>

--000000000000acba920618fb6e4f--