Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110668
MIME-Version: 1.0
References: <CANeKSiTAYFOUvA+V+Ytb=N91OPMv_iuJ1m6iFq2kPE80Qp0QJw@mail.gmail.com>
 <VI1PR02MB4703313585C4A88D85068462E29B0@VI1PR02MB4703.eurprd02.prod.outlook.com>
 <CANeKSiQjkMMWiA8K+xRGO3Bmdo+vpQcOQoOnCP2Oxya8i8AB3A@mail.gmail.com> <VI1PR02MB47037B983141D264874BB31BE29B0@VI1PR02MB4703.eurprd02.prod.outlook.com>
In-Reply-To: <VI1PR02MB47037B983141D264874BB31BE29B0@VI1PR02MB4703.eurprd02.prod.outlook.com>
Date: Thu, 18 Jun 2020 20:04:26 +0300
Message-ID: <CANeKSiTLb+UPUjUMnir8TW6GFQXu8nsp_qRsji3JO5e7o3ufUg@mail.gmail.com>
To: Bob Weinand <bobwei9@hotmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000021e26905a85ec6e5"
Subject: Re: [PHP-DEV] [RFC] [DISCUSSION] Make constructors and destructors
 return void
From: benas.molis.iml@gmail.com (Benas IML)

--00000000000021e26905a85ec6e5
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hey Bob,

Your examples (regarding constructors/destructors) do make sense since you
are using these methods for what they should be used: initialization and
freeing resources. But, this is off-topic to the RFC.

This RFC only proposes to enforce no-return rule on
constructors/destructors. And based on my previous reply, top 2,000
Composer packages that have a BC break don't actually return anything
(early returns and returns that don't change code behavior don't count).

Your examples regarding `__toString`, `__invoke` and `__serialize` are also
not relevant to this RFC. Although, I do understand that you were
responding to email and I'm thankful for that :)


Best regards,
Benas

On Thu, Jun 18, 2020, 7:43 PM Bob Weinand <bobwei9@hotmail.com> wrote:

> Hey,
>
> Am 18.06.2020 um 17:18 schrieb Benas IML <benas.molis.iml@gmail.com>:
>
> Hey Bob,
>
> Magic methods are **never** supposed to be called directly (even more if
> that method is a constructor or a destructor). If that's not the case, it=
's
> just plain bad code. But by enforcing these rules, we make sure that less
> of that (bad code) is written and as a result, we make PHP code less
> bug-prone and easier to debug. That's also most likely the reason why
>
>
> __construct() is invoked directly on parent calls, sometimes to
> reinitialize an object or
> after ReflectionClass::newInstanceWithoutConstructor.
>
> I invoke __destruct() directly when needing an early freeing of existing
> resources.
>
> "ensure magic methods' signature" RFC opted in to validate `__clone`
> method's signature and ensure that it has `void` return type.
>
> Just for the sake of making sure that you understand what I mean, here ar=
e
> a couple of examples that show that no magic method is ever supposed to b=
e
> called directly:
> ```php
> // __toString
> (string) $object;
>
>
> I like using ->__toString() in favor of (string) casts when the variable
> is guaranteed to be an object to highlight that and avoid magic-ness.
>
> // __invoke
> $object();
>
>
> Same here, unless the object is a closure.
>
> // __serialize
> serialize($object);
> ```
>
>
> Can't argue much about that one, I never use serialize().
>
> Moreover, by validating constructors/destructors and allowing an explicit
> `void` return type declaration, we are becoming much more consistent
> (something that PHP is striving for) with other magic methods (e. g.
> `__clone`).
>
>
> Yeah, __clone() is odd. No idea why.
>
> Also, saying that "sometimes you have valid information to pass from the
> parent class" is quite an overstatement. After analyzing most of the 95
> Composer packages that had a potential BC break, I found out that either
> they wanted to return early (that is still possible to do using `return;`=
)
> or they added a `return something;` for no reason. Thus, no libraries
> actually returned something useful and valid from a constructor (as they
> shouldn't).
>
> Last but certainly not least, constructors have one and only one
> responsibility - to initialize an object. Whether you read Wikipedia's or
> PHP manual's definition, a constructor does just that. It initializes. So=
,
> the PHP manual is perfectly correct and documents the correct return type
> that a constructor should have.
>
>
> It also is generally a bad idea to have side effects in constructors, but
> _sometimes_ it is justified. Only because something mostly is a bad idea,
> it is not always.
> Also note that other languages completely forbid manual ctor calls. But
> PHP doesn't (and for good reason, like after
> using ReflectionClass::newInstanceWithoutConstructor).
>
> Bob
>
> Best regards,
> Benas
>
> On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote:
>
>> > Am 17.06.2020 um 01:10 schrieb Benas IML <benas.molis.iml@gmail.com>:
>> >
>> > Hey internals,
>> >
>> > This is a completely refined, follow-up RFC to my original RFC. Based
>> on the
>> > feedback I have received, this PR implements full validation and
>> implicitly
>> > enforces `void` rules on constructors/destructors while also allowing =
to
>> > declare an **optional** explicit `void` return type. Note, that there
>> is a
>> > small but justifiable BC break (as stated by the RFC).
>> >
>> > RFC: https://wiki.php.net/rfc/make_ctor_ret_void
>> >
>> > Best regards,
>> > Benas Seliuginas
>>
>> Hey Benas,
>>
>> I do not see any particular benefit from that RFC.
>>
>> Regarding what the manual states - the manual is wrong there and thus
>> should be fixed in the manual. This is not an argument for changing engi=
ne
>> behaviour.
>>
>> Sometimes a constructor (esp. of a parent class) or destructor may be
>> called manually. Sometimes you have valid information to pass from the
>> parent class.
>> With your RFC an arbitrary restriction is introduced necessitating an
>> extra method instead.
>>
>> In general that RFC feels like "uh, __construct and __destruct are mostl=
y
>> void, so let's enforce it =E2=80=A6 because we can"?
>>
>> On these grounds and it being an additional (albeit mostly small)
>> unnecessary BC break, I'm not in favor of that RFC.
>>
>> Bob
>
>
>

--00000000000021e26905a85ec6e5--