Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121103
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-761-gece9e40c48-fm-20230913.001-gece9e40c
MIME-Version: 1.0
Message-ID: <5fe167d3-ceab-4ba5-8f33-6cb1c380c195@app.fastmail.com>
In-Reply-To: <39d49239-c87c-2c95-c293-f2952b5e3307@bastelstu.be>
References: <c7a7f77b-8187-23f7-7c02-c92f1b538039@gmail.com>
 <c774ef73-802b-42eb-9181-9a080713c390@gmail.com>
 <CAAwdEzCQps8+cFf86-6e_7gCXtdhG7DjLTyQ3TS-=3+kW-Pcqg@mail.gmail.com>
 <29eb53a7-9aef-4e48-999e-97574f27a9df@gmail.com>
 <4C6FD028-4E34-4578-AF04-EDAC120E3E94@koalephant.com>
 <b8665e13-d317-49a0-b740-b082ec6bd2c9@gmail.com>
 <872B9392-D91A-45BC-8956-3E53B16723A6@koalephant.com>
 <39d49239-c87c-2c95-c293-f2952b5e3307@bastelstu.be>
Date: Tue, 19 Sep 2023 17:42:50 +0000
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [RFC] [Discussion] DOM HTML5 parsing and serialization support
From: larry@garfieldtech.com ("Larry Garfield")

On Tue, Sep 19, 2023, at 7:30 AM, Tim D=C3=BCsterhus wrote:
> Hi
>
> On 9/19/23 08:35, Stephen Reay wrote:
>> Regarding the private constructor: I understand the issue with the *o=
ld* class being confusing - but your new class doesn't have that issue, =
because there are no "loadXXX" methods: as you said, if you're loading a=
n existing document, you're forced to do it via the static factory metho=
ds. With that change alone, there's zero risk of confusion in allowing d=
irect constructor calls, because once the object is instantiated there i=
s no subsequent way to load a document and inadvertently change the enco=
ding.
>>=20
>> Having a regular constructor and then one or more factory methods for=
 specific cases is already a concept in PHP (i.e. DateTime[Immutable]'s =
 `createFromXXX` as well as a constructor), and IMO needing to call a fa=
ctory method to get an "empty" document seems out of place in PHP - it s=
eems a bit like a Java-ism - using a factory, where one just isn't requi=
red.
>>=20
>
> I was one of the persons who discussed this updated API with Niels in=20
> private and looking up the discussion it was me who proposed making th=
e=20
> constructor private and just providing named constructors.
>
>  From the perspective of the user of the API, I like the symmetry=20
> between all the named constructors:
>
> Whenever I want to create a new document, I use one of the fromXyz()=20
> methods. And when I use those, I get exactly what it says on the tin.
>
> Making the regular constructor available for use would effectively mak=
e=20
> whatever that constructor does a special case / the default case. This=20
> makes sense for DateTimeImmutable, because the __construct() variant i=
s=20
> likely much more often used than the various createFromXyz() variants.=20
> For the HtmlDocument I find it less obvious that creating an empty=20
> document would be the default case compared to loading an existing=20
> document from a file.
>
> We should probably rename the named constructors to include the "creat=
e"=20
> prefix for consistency with DTI though.
>
> Best regards
> Tim D=C3=BCsterhus

I agree with Tim here.  If you have named constructors that are all "equ=
ally common," then it's non-obvious which one should be the "unnamed def=
ault."  I know that `fromX()` requires an X, `fromY()` requires a Y, but=
 what does `__construct()` require?  I cannot tell from the call site.  =
If __construct() is just an alias of on of the fromX() methods, then wha=
t purpose does it serve other than confusion?

In addition, although it's probably not super relevant here, static fact=
ory methods are chainable in a way that `new` is not.  No extra parens a=
re necessary.  For classes I'm creating at hoc (rather than services tha=
t are always managed via DI), I often prefer a named constructor just fo=
r convenience.

I don't see it as a Java-ism at all.  Java is (in)famous for excessive l=
ayers of factory classes, which is something different than what is goin=
g on here.  (How fair that reputation is in 2023, I don't know.)  This i=
s just "named constructors," which has been a reasonably common pattern =
in PHP land for a long time.

--Larry Garfield