Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121102
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.700.6\))
In-Reply-To: <39d49239-c87c-2c95-c293-f2952b5e3307@bastelstu.be>
Date: Tue, 19 Sep 2023 14:58:04 +0700
Cc: Niels Dossche <dossche.niels@gmail.com>,
 PHP Internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <C34FE3AE-B08B-44A8-A0E2-007726107C6B@koalephant.com>
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>
To: =?utf-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Subject: Re: [PHP-DEV] [RFC] [Discussion] DOM HTML5 parsing and serialization
 support
From: php-lists@koalephant.com (Stephen Reay)



> On 19 Sep 2023, at 14:30, Tim D=C3=BCsterhus <tim@bastelstu.be> wrote:
>=20
> Hi
>=20
> On 9/19/23 08:35, Stephen Reay wrote:
>> Regarding the private constructor: I understand the issue with the =
*old* 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 an existing document, you're forced to do it via the static =
factory methods. With that change alone, there's zero risk of confusion =
in allowing direct constructor calls, because once the object is =
instantiated there is no subsequent way to load a document and =
inadvertently change the encoding.
>> 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 =
factory method to get an "empty" document seems out of place in PHP - it =
seems a bit like a Java-ism - using a factory, where one just isn't =
required.
>=20
> I was one of the persons who discussed this updated API with Niels in =
private and looking up the discussion it was me who proposed making the =
constructor private and just providing named constructors.
>=20
> =46rom the perspective of the user of the API, I like the symmetry =
between all the named constructors:
>=20
> Whenever I want to create a new document, I use one of the fromXyz() =
methods. And when I use those, I get exactly what it says on the tin.
>=20
> Making the regular constructor available for use would effectively =
make whatever that constructor does a special case / the default case. =
This makes sense for DateTimeImmutable, because the __construct() =
variant is likely much more often used than the various createFromXyz() =
variants. For the HtmlDocument I find it less obvious that creating an =
empty document would be the default case compared to loading an existing =
document from a file.
>=20
> We should probably rename the named constructors to include the =
"create" prefix for consistency with DTI though.
>=20
> Best regards
> Tim D=C3=BCsterhus
>=20

Hi Tim,


Thanks for providing context on where this idea came from.


The constructor + specialised factory methods pattern is not exactly new =
in PHP (e.g. it took about 3 minutes to find multiple Symphony classes =
doing this), and disabling the public constructor for purely cosmetic =
reasons sounds like a very weird, and ironic choice to me, when the =
stated goal is to make the API *less surprising* than the previous one.


Cheers

Stephen=20=