Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121104
Message-ID: <dda71efd-df32-4780-a5d2-038bbe2ce96f@gmail.com>
Date: Tue, 19 Sep 2023 21:56:42 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Stephen Reay <php-lists@koalephant.com>
Cc: PHP Internals <internals@lists.php.net>
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>
Content-Language: en-US
In-Reply-To: <872B9392-D91A-45BC-8956-3E53B16723A6@koalephant.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] [Discussion] DOM HTML5 parsing and serialization
 support
From: dossche.niels@gmail.com (Niels Dossche)

Hi Stephen

On 19/09/2023 08:35, Stephen Reay wrote:
> 
>> On 19 Sep 2023, at 01:00, Niels Dossche 
>>
>> Cheers
>> Niels
>>
>> -- 
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: https://www.php.net/unsub.php
> 
> 
> 
> (Resending with history removed due to apparent size limit) 
> 
> Hi Niels,
> 
> Obviously, a method with different signatures (`fromEmptyDocument`) can't be in the parent, and I'll discuss that below, but having the other factory methods that do have the same signature in parent (even if only an abstract method, forcing concrete implementations to define it) means that it's possible to *safely* do meta-programming with these classes, where you load a string/file, using a classname from a variable. This use-case doesn't necessarily need to know which specific instance it's getting back, so long as it's an instance of the base class (or interface, if it had been one), but that information can still be presented, via the return type (either `static`, or `self` on the parent and the actual class name on each of the implementations).
> 

I don't really understand this part. What kind of meta-programming are you thinking of? Can you give an example?
Calling the create methods on the abstract type does not seem sensible, because the input data is not interchangeable between the two subclasses.
E.g. passing an XML input into HTMLDocument will not go well (and vice versa).
So it seems to me that you need to know the type to be able to give sensible input.

> 
> 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.
> 

I agree with what Tim and Larry have said here.
I do agree too that it should be called createFromString and createFromFile to be consistent with the DateTime classes.
And perhaps instead of createFromEmpty it should be createEmpty because you're not creating an instance "from" something here, you're creating it with "nothing".

> 
> Cheers
> 
> Stephen
> 

Kind regards
Niels