Re: [RFC] [Discussion] DOM HTML5 parsing and serialization support

> On 19 Sep 2023, at 14:30, Tim Düsterhus <tim@bastelstu.be> wrote: > > Hi > > 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 jòÈ,öŠ}¸j >> JA8ðÒ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. > > 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. > > From the perspective of the user of the API, I like the symmetry between all the named > constructors: > > 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. > > 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. > > We should probably rename the named constructors to include the "create" prefix for > consistency with DTI though. > > Best regards > Tim Düsterhus > 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