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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
