> On 19 Sep 2023, at 14:30, Tim Düsterhus <t...@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
>> `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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
